[PATCH] libnvdimm, region: hide persistence_domain when unknown

[Dan Williams]

Similar to other region attributes, do not emit the persistence_domain
attribute if its contents are empty.

Fixes: 96c3a239054a ("libnvdimm: expose platform persistence attr...")
Cc: Dave Jiang 
Signed-off-by: Dan Williams 
---
 drivers/nvdimm/region_devs.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index e6d01911e092..a8e9d428c0a5 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -593,6 +593,13 @@ static umode_t region_visible(struct kobject *kobj, struct 
attribute *a, int n)
return 0;
}
 
+   if (a == _attr_persistence_domain.attr) {
+   if ((nd_region->flags & (BIT(ND_REGION_PERSIST_CACHE)
+   | BIT(ND_REGION_PERSIST_MEMCTRL))) == 0)
+   return 0;
+   return a->mode;
+   }
+
if (a != _attr_set_cookie.attr
&& a != _attr_available_size.attr)
return a->mode;

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

Returned mail: see transcript for details

[Bounced mail]

The original message was received at Wed, 21 Mar 2018 11:58:25 +0800
from lists.01.org [26.168.133.30]

- The following addresses had permanent fatal errors -


- Transcript of session follows -
  while talking to lists.01.org.:
>>> MAIL From:"Bounced mail" 
<<< 501 "Bounced mail" ... Refused



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

Re: [PATCH v5] ndctl: Add support for get bus and region persistence domain

[Dan Williams]

On Tue, Mar 20, 2018 at 3:50 PM, Dave Jiang  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 
> ---
> 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(_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(>regions, >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 

Returned mail: Data format error

[brett]

This Message was undeliverable due to the following reason:

Your message was not delivered because the destination computer was
not reachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message was not delivered within 6 days:
Host 204.208.68.189 is not responding.

The following recipients did not receive this message:


Please reply to postmas...@jdom.org
if you feel this message to be in error.



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

Re: [PATCH v4] ndctl: Add support for get bus and region persistence domain

[Dave Jiang]

On 03/20/2018 03:20 PM, Ross Zwisler wrote:
> On Tue, Mar 20, 2018 at 03:02:59PM -0700, Dave Jiang 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 
>> ---
>>
>> v4:
>> - change enum ndctl_persistence to enum ndctl_persistence_domain
> 
> I think you missed Dan's second comment in that mail about spacing out the
> enum ndctl_persistence_domain members.
> 

Ah yeah I must've have. How should we space that out?
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v4] ndctl: Add support for get bus and region persistence domain

[Ross Zwisler]

On Tue, Mar 20, 2018 at 03:02:59PM -0700, Dave Jiang 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 
> ---
> 
> v4:
> - change enum ndctl_persistence to enum ndctl_persistence_domain

I think you missed Dan's second comment in that mail about spacing out the
enum ndctl_persistence_domain members.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v4] ndctl: Add support for get bus and region persistence domain

[Dave Jiang]

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 
---

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(_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);
+
+   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(>regions, >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
--- 

Re: [PATCH v3] ndctl: Add support for get bus and region persistence domain

[Dan Williams]

On Tue, Mar 20, 2018 at 2:50 PM, Dave Jiang  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 
> ---
>
> 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   |   11 ++
>  ndctl/list.c   |   16 +
>  4 files changed, 114 insertions(+)
[..]
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index f3a27411..84f441ef 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -115,6 +115,7 @@ 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 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 +306,14 @@ struct badblock {
> unsigned long long offset;
> unsigned int len;
>  };
> +
> +enum ndctl_persistence {

Let's fully spell this out this type as "enum ndctl_persistence_domain"

> +   PERSISTENCE_NONE = 0,
> +   PERSISTENCE_MEM_CTRL,
> +   PERSISTENCE_CPU_CACHE,
> +   PERSISTENCE_UNKNOWN,
> +};

Just in case there might be additional theoretical persistence domains
in the future between cache and memory controller, or memory
controller and media lets pad some values into this definition.

I.e. something like:

   PERSISTENCE_NONE = 0,
   PERSISTENCE_MEM_CTRL = 10,
   PERSISTENCE_CPU_CACHE = 20,
   PERSISTENCE_UNKNOWN = INT_MAX,
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v3] ndctl: Add support for get bus and region persistence domain

[Dave Jiang]

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 
---

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   |   11 ++
 ndctl/list.c   |   16 +
 4 files changed, 114 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index a165e697..bd2afa01 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 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
+ndctl_bus_get_persistence_domain(struct ndctl_bus *bus)
+{
+   struct ndctl_region *region;
+   enum ndctl_persistence 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 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 pd = PERSISTENCE_NONE;
+
+   region->persistence_domain = PERSISTENCE_NONE;
+   if (asprintf(_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);
+
+   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(>regions, >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
+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;
 

[PATCH] libndctl: adjusting debug output for return values of a command sent

[Dave Jiang]

Moving the success indicator next to status output to make it easier to
read since that is correlated to status and not firmware status output.
Also changing the firmware status to hex in order to reflect the spec.

Signed-off-by: Dave Jiang 
---
 ndctl/lib/libndctl.c |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index a165e697..413a7db3 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2441,11 +2441,11 @@ static int do_cmd(int fd, int ioctl_cmd, struct 
ndctl_cmd *cmd)
 
if (iter->total_xfer == 0) {
rc = ioctl(fd, ioctl_cmd, cmd->cmd_buf);
-   dbg(ctx, "bus: %d dimm: %#x cmd: %s%s%s status: %d fw: %d 
(%s)\n",
+   dbg(ctx, "bus: %d dimm: %#x cmd: %s%s%s status: %d (%s) fw: 
%#x\n",
bus->id, dimm ? ndctl_dimm_get_handle(dimm) : 0,
name, sub_name ? ":" : "", sub_name ? sub_name 
: "",
-   rc, *(cmd->firmware_status), rc < 0 ?
-   strerror(errno) : "success");
+   rc, rc < 0 ?  strerror(errno) : "success",
+   *(cmd->firmware_status));
if (rc < 0)
return -errno;
else
@@ -2474,12 +2474,12 @@ static int do_cmd(int fd, int ioctl_cmd, struct 
ndctl_cmd *cmd)
}
}
 
-   dbg(ctx, "bus: %d dimm: %#x cmd: %s%s%s total: %d max_xfer: %d status: 
%d fw: %d (%s)\n",
+   dbg(ctx, "bus: %d dimm: %#x cmd: %s%s%s total: %d max_xfer: %d status: 
%d (%s) fw: %#x\n",
bus->id, dimm ? ndctl_dimm_get_handle(dimm) : 0,
name, sub_name ? ":" : "", sub_name ? sub_name : "",
iter->total_xfer, iter->max_xfer, rc,
-   *(cmd->firmware_status),
-   rc < 0 ? strerror(errno) : "success");
+   rc < 0 ? strerror(errno) : "success",
+   *(cmd->firmware_status));
 
return rc;
 }

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

[ndctl PATCH] ndctl, sysfs: null terminate buffer on error

[Dan Williams]

Terminate the buffer for sysfs_read_attr() users that might not
properly handle the error code.

Signed-off-by: Dan Williams 
---
 util/sysfs.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/util/sysfs.c b/util/sysfs.c
index e067f065a820..0440fd0f49a3 100644
--- a/util/sysfs.c
+++ b/util/sysfs.c
@@ -39,6 +39,7 @@ int __sysfs_read_attr(struct log_ctx *ctx, const char *path, 
char *buf)
n = read(fd, buf, SYSFS_ATTR_SIZE);
close(fd);
if (n < 0 || n >= SYSFS_ATTR_SIZE) {
+   buf[0] = 0;
log_dbg(ctx, "failed to read %s: %s\n", path, strerror(errno));
return -errno;
}

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

Re: [PATCH v2] ndctl: Add support for get bus and region persistent domain

[Dan Williams]

On Mon, Mar 19, 2018 at 4:45 PM, Dave Jiang  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 
> ---
> v2:
> - Simplied scanning of persistence domain from Ross's comments.
>
>  ndctl/lib/libndctl.c   |   87 
> 
>  ndctl/lib/libndctl.sym |2 +
>  ndctl/lib/private.h|1 +
>  ndctl/libndctl.h   |   10 ++
>  ndctl/list.c   |   16 +
>  5 files changed, 116 insertions(+)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index a165e697..8f4e1745 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 persistence_domain;
>  };
>
>  /**
> @@ -755,6 +756,7 @@ static void *add_bus(void *parent, int id, const char 
> *ctl_base)
> list_head_init(>regions);
> bus->ctx = ctx;
> bus->id = id;
> +   bus->persistence_domain = PERSISTENCE_UNKNOWN;
>
> sprintf(path, "%s/dev", ctl_base);
> if (sysfs_read_attr(ctx, path, buf) < 0
> @@ -916,6 +918,17 @@ NDCTL_EXPORT struct ndctl_bus 
> *ndctl_bus_get_by_provider(struct ndctl_ctx *ctx,
> return NULL;
>  }
>
> +NDCTL_EXPORT unsigned int
> +ndctl_bus_get_persistence_domain(struct ndctl_bus *bus)
> +{
> +   struct ndctl_region *region;
> +
> +   /* iterate through region to get the region persistence domain */
> +   ndctl_region_foreach(bus, region) {}
> +
> +   return bus->persistence_domain;

There's no need to add this property to the 'struct ndctl_bus' object.
Just do the aggregation live in the ndctl_region_foreach() loop that
you already have.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2] ndctl: Add support for get bus and region persistent domain

[Ross Zwisler]

On Mon, Mar 19, 2018 at 04:45:30PM -0700, Dave Jiang 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 

Nice this is cleaner, and you've shaved off 20 lines. :)

> @@ -755,6 +756,7 @@ static void *add_bus(void *parent, int id, const char 
> *ctl_base)
>   list_head_init(>regions);
>   bus->ctx = ctx;
>   bus->id = id;
> + bus->persistence_domain = PERSISTENCE_UNKNOWN;
>  
>   sprintf(path, "%s/dev", ctl_base);
>   if (sysfs_read_attr(ctx, path, buf) < 0
> @@ -916,6 +918,17 @@ NDCTL_EXPORT struct ndctl_bus 
> *ndctl_bus_get_by_provider(struct ndctl_ctx *ctx,
>   return NULL;
>  }
>  
> +NDCTL_EXPORT unsigned int

Can we make this return an enum ndctl_persistence, like the internal functions
do?  This follows the lead of APIs like ndctl_namespace_get_mode(), which by
returning an enum ndctl_namespace_mode tells the user what values to expect
and what those values mean.

Ditto for the ndctl_region_get_persistence_domain(), and the two external
declarations in libndctl.h.

> +ndctl_bus_get_persistence_domain(struct ndctl_bus *bus)
> +{
> + struct ndctl_region *region;
> +
> + /* iterate through region to get the region persistence domain */
> + ndctl_region_foreach(bus, region) {}

I still don't understand why you need to do a ndctl_region_foreach() here,
looping through all the regions on a bus?  I think you're just after the
initialization that

ndctl_region_foreach()
  ndctl_region_get_first()
regions_init()

gives you, right?  If so, let's be more clear about that and just call
regions_init() directly.  This also saves the work of needlessly looping
through the regions, and makes it so you don't need a local 'region' variable.


> +
> + return bus->persistence_domain;
> +}
> +
>  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 +1768,62 @@ static int region_set_type(struct ndctl_region 
> *region, char *path)
>   return 0;
>  }
>  
> +static enum ndctl_persistence 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 pd = PERSISTENCE_NONE;
> +
> + if (asprintf(_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;
> + }
> +
> + region->persistence_domain = PERSISTENCE_NONE;

I think that this initialization needs to happen at the top of the function
before we have any return paths.  This is because the code that calls this,
add_region(), doesn't set region->persistence_domain if we give an error
return.  I think that this means that if the asprintf() fails, for example, we
will end up with region->persistence_domain being uninitialized.


> + 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);
> +
> + 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];
> @@ -1762,6 +1831,7 @@ static void *add_region(void *parent, int id, const 
> char *region_base)
>   struct ndctl_bus *bus = parent;
>   struct ndctl_ctx *ctx = bus->ctx;
>   char *path = calloc(1, strlen(region_base) + 100);
> + int rc;
>  
>   if (!path)
>   return NULL;
> @@ -1831,6 +1901,17 @@ static void *add_region(void *parent, int 

Returned mail: see transcript for details

[MAILER-DAEMON]

The original message was included as attachment

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