| Message ID | 166777844020.1238089.5777920571190091563.stgit@dwillia2-xfh.jf.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl-cli test and usability updates | expand |
On Sun, Nov 06, 2022 at 03:47:20PM -0800, Dan Williams wrote: > The typical case is that CXL devices are pure ram devices. Only emit > capacity sizes when they are non-zero to avoid confusion around whether > pmem is available via partitioning or not. > > Do the same for ram_size on the odd case that someone builds a pure pmem > device. Maybe a few more words around what confusion this seeks to avoid. The confusion being that a user may assign more meaning to the zero size value than it actually deserves. A zero value for either pmem or ram, doesn't indicate the devices capability for either mode. Use the -I option to cxl list to include paritition info in the memdev listing. That will explicitly show the ram and pmem capabilities of the device. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > cxl/json.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/cxl/json.c b/cxl/json.c > index 63c17519aba1..1b1669ab021d 100644 > --- a/cxl/json.c > +++ b/cxl/json.c > @@ -305,7 +305,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev, > { > const char *devname = cxl_memdev_get_devname(memdev); > struct json_object *jdev, *jobj; > - unsigned long long serial; > + unsigned long long serial, size; > int numa_node; > > jdev = json_object_new_object(); > @@ -316,13 +316,19 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev, > if (jobj) > json_object_object_add(jdev, "memdev", jobj); > > - jobj = util_json_object_size(cxl_memdev_get_pmem_size(memdev), flags); > - if (jobj) > - json_object_object_add(jdev, "pmem_size", jobj); > + size = cxl_memdev_get_pmem_size(memdev); > + if (size) { > + jobj = util_json_object_size(size, flags); > + if (jobj) > + json_object_object_add(jdev, "pmem_size", jobj); > + } > > - jobj = util_json_object_size(cxl_memdev_get_ram_size(memdev), flags); > - if (jobj) > - json_object_object_add(jdev, "ram_size", jobj); > + size = cxl_memdev_get_ram_size(memdev); > + if (size) { > + jobj = util_json_object_size(size, flags); > + if (jobj) > + json_object_object_add(jdev, "ram_size", jobj); > + } > > if (flags & UTIL_JSON_HEALTH) { > jobj = util_cxl_memdev_health_to_json(memdev, flags); >
On Sun, Nov 06, 2022 at 03:47:20PM -0800, Dan Williams wrote: > The typical case is that CXL devices are pure ram devices. Only emit > capacity sizes when they are non-zero to avoid confusion around whether > pmem is available via partitioning or not. > > Do the same for ram_size on the odd case that someone builds a pure pmem > device. The cxl list man page needs a couple of examples updated. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > cxl/json.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/cxl/json.c b/cxl/json.c > index 63c17519aba1..1b1669ab021d 100644 > --- a/cxl/json.c > +++ b/cxl/json.c > @@ -305,7 +305,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev, > { > const char *devname = cxl_memdev_get_devname(memdev); > struct json_object *jdev, *jobj; > - unsigned long long serial; > + unsigned long long serial, size; > int numa_node; > > jdev = json_object_new_object(); > @@ -316,13 +316,19 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev, > if (jobj) > json_object_object_add(jdev, "memdev", jobj); > > - jobj = util_json_object_size(cxl_memdev_get_pmem_size(memdev), flags); > - if (jobj) > - json_object_object_add(jdev, "pmem_size", jobj); > + size = cxl_memdev_get_pmem_size(memdev); > + if (size) { > + jobj = util_json_object_size(size, flags); > + if (jobj) > + json_object_object_add(jdev, "pmem_size", jobj); > + } > > - jobj = util_json_object_size(cxl_memdev_get_ram_size(memdev), flags); > - if (jobj) > - json_object_object_add(jdev, "ram_size", jobj); > + size = cxl_memdev_get_ram_size(memdev); > + if (size) { > + jobj = util_json_object_size(size, flags); > + if (jobj) > + json_object_object_add(jdev, "ram_size", jobj); > + } > > if (flags & UTIL_JSON_HEALTH) { > jobj = util_cxl_memdev_health_to_json(memdev, flags); >
Alison Schofield wrote: > On Sun, Nov 06, 2022 at 03:47:20PM -0800, Dan Williams wrote: > > The typical case is that CXL devices are pure ram devices. Only emit > > capacity sizes when they are non-zero to avoid confusion around whether > > pmem is available via partitioning or not. > > > > Do the same for ram_size on the odd case that someone builds a pure pmem > > device. > > Maybe a few more words around what confusion this seeks to avoid. > The confusion being that a user may assign more meaning to the zero > size value than it actually deserves. A zero value for either > pmem or ram, doesn't indicate the devices capability for either mode. > Use the -I option to cxl list to include paritition info in the > memdev listing. That will explicitly show the ram and pmem capabilities > of the device. Nice, I like it. Will append for v2, or maybe Vishal can grab it when applying? Depends on if I need to respin other patches.
Alison Schofield wrote: > On Sun, Nov 06, 2022 at 03:47:20PM -0800, Dan Williams wrote: > > The typical case is that CXL devices are pure ram devices. Only emit > > capacity sizes when they are non-zero to avoid confusion around whether > > pmem is available via partitioning or not. > > > > Do the same for ram_size on the odd case that someone builds a pure pmem > > device. > > The cxl list man page needs a couple of examples updated. Ah, good catch, will fix.
Alison Schofield wrote: > On Sun, Nov 06, 2022 at 03:47:20PM -0800, Dan Williams wrote: > > The typical case is that CXL devices are pure ram devices. Only emit > > capacity sizes when they are non-zero to avoid confusion around whether > > pmem is available via partitioning or not. > > > > Do the same for ram_size on the odd case that someone builds a pure pmem > > device. > > Maybe a few more words around what confusion this seeks to avoid. > The confusion being that a user may assign more meaning to the zero > size value than it actually deserves. A zero value for either > pmem or ram, doesn't indicate the devices capability for either mode. > Use the -I option to cxl list to include paritition info in the > memdev listing. That will explicitly show the ram and pmem capabilities > of the device. > I went ahead and added this verbatim to the changelog, thanks!
Alison Schofield wrote: > On Sun, Nov 06, 2022 at 03:47:20PM -0800, Dan Williams wrote: > > The typical case is that CXL devices are pure ram devices. Only emit > > capacity sizes when they are non-zero to avoid confusion around whether > > pmem is available via partitioning or not. > > > > Do the same for ram_size on the odd case that someone builds a pure pmem > > device. > > The cxl list man page needs a couple of examples updated. Good point, done.
diff --git a/cxl/json.c b/cxl/json.c index 63c17519aba1..1b1669ab021d 100644 --- a/cxl/json.c +++ b/cxl/json.c @@ -305,7 +305,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev, { const char *devname = cxl_memdev_get_devname(memdev); struct json_object *jdev, *jobj; - unsigned long long serial; + unsigned long long serial, size; int numa_node; jdev = json_object_new_object(); @@ -316,13 +316,19 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev, if (jobj) json_object_object_add(jdev, "memdev", jobj); - jobj = util_json_object_size(cxl_memdev_get_pmem_size(memdev), flags); - if (jobj) - json_object_object_add(jdev, "pmem_size", jobj); + size = cxl_memdev_get_pmem_size(memdev); + if (size) { + jobj = util_json_object_size(size, flags); + if (jobj) + json_object_object_add(jdev, "pmem_size", jobj); + } - jobj = util_json_object_size(cxl_memdev_get_ram_size(memdev), flags); - if (jobj) - json_object_object_add(jdev, "ram_size", jobj); + size = cxl_memdev_get_ram_size(memdev); + if (size) { + jobj = util_json_object_size(size, flags); + if (jobj) + json_object_object_add(jdev, "ram_size", jobj); + } if (flags & UTIL_JSON_HEALTH) { jobj = util_cxl_memdev_health_to_json(memdev, flags);
The typical case is that CXL devices are pure ram devices. Only emit capacity sizes when they are non-zero to avoid confusion around whether pmem is available via partitioning or not. Do the same for ram_size on the odd case that someone builds a pure pmem device. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- cxl/json.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)