On Tue, Mar 20, 2018 at 3:50 PM, Dave Jiang <dave.ji...@intel.com> wrote: > Adding helper functions to iterate through sysfs region persistence domain > attribute. The region will display the domain with the most persistence for > the > region. The bus will display the domain attribute with the least persistence > amongst all the regions. ndctl_bus_get_persistence_domain() and > ndctl_region_get_persistence_domain are exported. ndctl list will also display > the region persistence domain as well. > > Signed-off-by: Dave Jiang <dave.ji...@intel.com> > --- > v5: > - space out ndctl_persistence_domain for future attributes > > v4: > - change enum ndctl_persistence to enum ndctl_persistence_domain > > v3: > - fixed up return types per Ross's comments > - removed persistence_domain for bus and calculate on the fly per Dan's > comment > > v2: > - Simplied scanning of persistence domain from Ross's comments. > > > ndctl/lib/libndctl.c | 85 > ++++++++++++++++++++++++++++++++++++++++++++++++ > ndctl/lib/libndctl.sym | 2 + > ndctl/libndctl.h | 12 +++++++ > ndctl/list.c | 16 +++++++++ > 4 files changed, 115 insertions(+) > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c > index a165e697..d97f1501 100644 > --- a/ndctl/lib/libndctl.c > +++ b/ndctl/lib/libndctl.c > @@ -180,6 +180,7 @@ struct ndctl_region { > } iset; > FILE *badblocks; > struct badblock bb; > + enum ndctl_persistence_domain persistence_domain; > }; > > /** > @@ -916,6 +917,22 @@ NDCTL_EXPORT struct ndctl_bus > *ndctl_bus_get_by_provider(struct ndctl_ctx *ctx, > return NULL; > } > > +NDCTL_EXPORT enum ndctl_persistence_domain > +ndctl_bus_get_persistence_domain(struct ndctl_bus *bus) > +{ > + struct ndctl_region *region; > + enum ndctl_persistence_domain pd = PERSISTENCE_UNKNOWN; > + > + /* iterate through region to get the region persistence domain */ > + ndctl_region_foreach(bus, region) { > + /* we are looking for the least persistence domain */ > + if (pd > region->persistence_domain) > + pd = region->persistence_domain; > + } > + > + return pd; > +} > + > NDCTL_EXPORT struct ndctl_btt *ndctl_region_get_btt_seed(struct ndctl_region > *region) > { > struct ndctl_ctx *ctx = ndctl_region_get_ctx(region); > @@ -1755,6 +1772,62 @@ static int region_set_type(struct ndctl_region > *region, char *path) > return 0; > } > > +static enum ndctl_persistence_domain region_get_pd_type(char *name) > +{ > + if (strncmp("cpu_cache", name, 9) == 0) > + return PERSISTENCE_CPU_CACHE; > + else if (strncmp("memory_controller", name, 17) == 0) > + return PERSISTENCE_MEM_CTRL; > + else > + return PERSISTENCE_UNKNOWN; > +} > + > +static int region_persistence_scan(struct ndctl_region *region) > +{ > + struct ndctl_ctx *ctx = ndctl_region_get_ctx(region); > + char *pd_path; > + FILE *pf; > + char buf[64]; > + int rc = 0; > + enum ndctl_persistence_domain pd = PERSISTENCE_NONE; > + > + region->persistence_domain = PERSISTENCE_NONE; > + if (asprintf(&pd_path, "%s/persistence_domain", > + region->region_path) < 0) { > + rc = -errno; > + err(ctx, "region persist domain path allocation failure\n"); > + return rc; > + } > + > + pf = fopen(pd_path, "re"); > + if (!pf) { > + rc = -errno; > + free(pd_path); > + return rc; > + } > + > + do { > + rc = fscanf(pf, "%s", buf); > + if (rc == EOF) { > + if (ferror(pf)) { > + rc = -errno; > + goto out; > + } > + } else if (rc == 1) > + pd = region_get_pd_type(buf); > + > + if (region->persistence_domain < pd) > + region->persistence_domain = pd; > + } while (rc != EOF);
I would expect sysfs_read_attr() here? I don't otherwise see a reason to have special case code for this attribute. > + > + rc = 0; > + > +out: > + fclose(pf); > + free(pd_path); > + return rc; > +} > + > static void *add_region(void *parent, int id, const char *region_base) > { > char buf[SYSFS_ATTR_SIZE]; > @@ -1831,6 +1904,12 @@ static void *add_region(void *parent, int id, const > char *region_base) > list_add(&bus->regions, ®ion->list); > > free(path); > + > + /* get the persistence domain attribs */ > + if (region_persistence_scan(region) < 0) > + err(ctx, "%s: region persistence scan failed\n", > + ndctl_region_get_devname(region)); > + > return region; > > err_read: > @@ -2093,6 +2172,12 @@ NDCTL_EXPORT struct badblock > *ndctl_region_get_first_badblock(struct ndctl_regio > return ndctl_region_get_next_badblock(region); > } > > +NDCTL_EXPORT enum ndctl_persistence_domain > +ndctl_region_get_persistence_domain(struct ndctl_region *region) > +{ > + return region->persistence_domain; > +} > + > static struct nd_cmd_vendor_tail *to_vendor_tail(struct ndctl_cmd *cmd) > { > struct nd_cmd_vendor_tail *tail = (struct nd_cmd_vendor_tail *) > diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym > index 21276614..3209aefe 100644 > --- a/ndctl/lib/libndctl.sym > +++ b/ndctl/lib/libndctl.sym > @@ -350,4 +350,6 @@ global: > ndctl_dimm_cmd_new_ack_shutdown_count; > ndctl_region_get_numa_node; > ndctl_dimm_fw_update_supported; > + ndctl_region_get_persistence_domain; > + ndctl_bus_get_persistence_domain; > } LIBNDCTL_14; > diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h > index f3a27411..eaa3af5f 100644 > --- a/ndctl/libndctl.h > +++ b/ndctl/libndctl.h > @@ -115,6 +115,8 @@ int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus, int > cmd); > unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus); > unsigned int ndctl_bus_get_id(struct ndctl_bus *bus); > const char *ndctl_bus_get_provider(struct ndctl_bus *bus); > +enum ndctl_persistence_domain ndctl_bus_get_persistence_domain( > + struct ndctl_bus *bus); > int ndctl_bus_wait_probe(struct ndctl_bus *bus); > int ndctl_bus_wait_for_scrub_completion(struct ndctl_bus *bus); > unsigned int ndctl_bus_get_scrub_count(struct ndctl_bus *bus); > @@ -305,6 +307,14 @@ struct badblock { > unsigned long long offset; > unsigned int len; > }; > + > +enum ndctl_persistence_domain { > + PERSISTENCE_NONE = 0, > + PERSISTENCE_MEM_CTRL = 10, > + PERSISTENCE_CPU_CACHE = 20, > + PERSISTENCE_UNKNOWN = INT32_MAX, Why INT32_MAX and not INT_MAX? Let's stick to limits.h definitions. > +}; > + > struct ndctl_region; > struct ndctl_region *ndctl_region_get_first(struct ndctl_bus *bus); > struct ndctl_region *ndctl_region_get_next(struct ndctl_region *region); > @@ -347,6 +357,8 @@ struct ndctl_region > *ndctl_bus_get_region_by_physical_address(struct ndctl_bus * > for (dimm = ndctl_region_get_first_dimm(region); \ > dimm != NULL; \ > dimm = ndctl_region_get_next_dimm(region, dimm)) > +enum ndctl_persistence_domain ndctl_region_get_persistence_domain( > + struct ndctl_region *region); > int ndctl_region_is_enabled(struct ndctl_region *region); > int ndctl_region_enable(struct ndctl_region *region); > int ndctl_region_disable_invalidate(struct ndctl_region *region); > diff --git a/ndctl/list.c b/ndctl/list.c > index fe8036ea..c9f4b74d 100644 > --- a/ndctl/list.c > +++ b/ndctl/list.c > @@ -73,6 +73,7 @@ static struct json_object *region_to_json(struct > ndctl_region *region, > struct ndctl_interleave_set *iset; > struct ndctl_mapping *mapping; > unsigned int bb_count = 0; > + enum ndctl_persistence_domain pd; > int numa; > > if (!jregion) > @@ -174,6 +175,21 @@ static struct json_object *region_to_json(struct > ndctl_region *region, > if ((flags & UTIL_JSON_MEDIA_ERRORS) && jbbs) > json_object_object_add(jregion, "badblocks", jbbs); > > + pd = ndctl_region_get_persistence_domain(region); > + switch (pd) { > + case PERSISTENCE_CPU_CACHE: Small nit, please use kernel coding style and line up the 'case' with the 'switch'. > + jobj = json_object_new_string("cpu_cache"); > + break; > + case PERSISTENCE_MEM_CTRL: > + jobj = json_object_new_string("memory_controller"); > + break; > + default: > + jobj = NULL; Let's go ahead and print "unknown" for this case. I want people using fake pmem to realize it, and otherwise encourage BIOS implementations to publish the platform capabilities table to get rid of the scary '"persistence_domain":"unknown"' messages. This would also let us deprecate the kernel warning message about persistence which is easy to overlook. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm