4:51 p.m.
> Hi Qi,
>
> This is getting closer to where I want to see this go, but still some
> architecture details to incorporate. I mentioned on the cover letter
> that systemd can handle starting, stopping, and show the status of
> the
> monitor. The other detail to incorporate is that monitor events can
> come DIMMs, but also namespaces, regions, and the bus.
>
> The event list I have collected to date is:
>
> dimm-spares-remaining
> dimm-media-temperature
> dimm-controller-temperature
> dimm-health-state
> dimm-unclean-shutdown
> dimm-detected
> namespace-media-error
> namespace-detected
> region-media-error
> region-detected
> bus-media-error
> bus-address-range-scrub-complete
> bus-detected
By referring to dimm_listen() in smart-listen.c, I understand how
to
monitor events
come DIMMs. I have checked the ndctl, but I could not find how to
monitor events
come namespaces, region and bus. Could you please mention more?
> ...and I think all of those should be separate options, probably
> something like the following, but I'd Vishal to comment if this
> scheme
> can be handled with the bash tab-completion implementation:
>
> ndctl monitor --dimm-events=spares-remaining,media-temperature
> --namespace-events=all --regions-events --bus=ACPI.NFIT
Yes I think we should be able to extend bash completion for a comma
separated list of arguments.
> ...where an empty --<object>-events option is equivalent to
> --<object>-events=all. Also, similar to "ndctl list" specifying
> specific buses, namespaces, etc causes the monitor to filter the
> objects based on those properties.
>
> Since "ndctl list" already has this filtering implemented I'd like to
> see it refactored and shared between the 2 implementations rather
> than
> duplicated as is done in this patch. In other words rework cmd_list()
> into a generic nvdimm object walking routine with callback functions
> to 'list' or 'monitor' a given object that matches the filter.
>
> Let me know if the above makes sense. I'm thinking the 'ndctl list'
> refactoring might be something I need to handle.
Yes, I agree with refactoring
ndctl_list and add a structure shared between
ndctl list and ndctl monitor.
But I prefer to use tokens rather than callback functions.
I also think the option should support multiple parameters likes:
ndclt monitor --dimm nmem1,nmem2 --region region1,region2
The parameters from the same option can be stored in a list_node.
#define filter_node(field) \
struct filter_##field##_node { \
const char *name; \
struct list_node list; \
};
filter_node(bus);
filter_node(dimm);
filter_node(region);
filter_node(namespace);
struct ndctl_filter {
struct list_head filter_bus;
struct list_head filter_dimm;
struct list_head filter_region;
struct list_head filter_namespace;
} nf;
Make the tokens like follows:
#define list_util_field_filter(field) \
list_for_each(&nf.filter_##field, filter_##field##_node, list) { \
if (util_##field##_filter(field, filter_##field##_node->name)) { \
flag = true; \
break; \
} \
}
#define list_util_bus_filter_by_field(field) \
list_for_each(&nf.filter_##field, filter_##field##_node, list) { \
if (util_bus_filter_by_##field(bus,
filter_##field##_node->name)) { \
flag = true; \
break; \
} \
}
#define list_util_dimm_filter_by_field(field) \
list_for_each(&nf.filter_##field, filter_##field##_node, list) { \
if (util_dimm_filter_by_##field(dimm,
filter_##field##_node->name)) { \
flag = true; \
break; \
} \
}
Here is how to use the tokens:
ndctl_bus_foreach(ctx, bus) {
struct filter_bus_node *filter_bus_node;
list_util_field_filter(bus);
if (!flag) {
struct filter_region_node *filter_region_node;
list_util_bus_filter_by_field(region);
}
if (!flag) {
struct filter_namespace_node *filter_namespace_node;
list_util_bus_filter_by_field(namespace);
}
if (!flag)
continue;
ndctl_dimm_foreach(bus, dimm) {
flag = false;
struct filter_dimm_node *filter_dimm_node;
list_util_field_filter(dimm);
if (!flag) {
struct filter_region_node *filter_region_node;
list_util_dimm_filter_by_field(region);
}
if (!flag) {
struct filter_namespace_node
*filter_namespace_node;
list_util_dimm_filter_by_field(namespace);
}
if (!flag)
continue;
}
}
To make sure the util_ndctl_filter() goes right, I would like you give
some comments?