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?

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to