| Message ID | 20190717225400.9494-6-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | daxctl: add a new reconfigure-device command | expand |
On Wed, Jul 17, 2019 at 3:54 PM Vishal Verma <vishal.l.verma@intel.com> wrote: > > The kernel provides a 'target_node' attribute for dax devices. When > converting a dax device to the system-ram mode, the memory is hotplugged > into this numa node. It would be helpful to print this in device > listings so that it is easy for applications to detect the node to > which the new memory belongs. > > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > util/json.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/util/json.c b/util/json.c > index babdc8c..f521337 100644 > --- a/util/json.c > +++ b/util/json.c > @@ -271,6 +271,7 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev, > { > const char *devname = daxctl_dev_get_devname(dev); > struct json_object *jdev, *jobj; > + int node; > > jdev = json_object_new_object(); > if (!devname || !jdev) > @@ -284,6 +285,13 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev, > if (jobj) > json_object_object_add(jdev, "size", jobj); > > + node = daxctl_dev_get_target_node(dev); > + if (node >= 0) { > + jobj = json_object_new_int(node); > + if (jobj) > + json_object_object_add(jdev, "target_node", jobj); > + } > + We moved 'numa_node' to the UTIL_JSON_VERBOSE set on "ndctl list" should do the same for target node?
On Thu, 2019-07-18 at 16:41 -0700, Dan Williams wrote: > On Wed, Jul 17, 2019 at 3:54 PM Vishal Verma <vishal.l.verma@intel.com > > wrote: > > > > @@ -284,6 +285,13 @@ struct json_object > > *util_daxctl_dev_to_json(struct daxctl_dev *dev, > > if (jobj) > > json_object_object_add(jdev, "size", jobj); > > > > + node = daxctl_dev_get_target_node(dev); > > + if (node >= 0) { > > + jobj = json_object_new_int(node); > > + if (jobj) > > + json_object_object_add(jdev, "target_node", > > jobj); > > + } > > + > > We moved 'numa_node' to the UTIL_JSON_VERBOSE set on "ndctl list" > should do the same for target node? Hm, true. Arguably, the target_node is much more pertinent in system-ram mode, and /should/ be in the default verbosity? One option could be to make it always show if the mode is system-ram, but not otherwise - but I don't know if that would cause more confusion as an attribute might seem to magically appear or disappear with the same command options.. Yet another option is, the output right after daxctl-reconfigure-device always sets UTIL_JSON_VERBOSE, but for daxctl-list, it is only done if the user supplies it. Any preferences on which way to go? Thanks, -Vishal
On Fri, Jul 19, 2019 at 11:08 AM Verma, Vishal L <vishal.l.verma@intel.com> wrote: > > On Thu, 2019-07-18 at 16:41 -0700, Dan Williams wrote: > > On Wed, Jul 17, 2019 at 3:54 PM Vishal Verma <vishal.l.verma@intel.com > > > wrote: > > > > > > @@ -284,6 +285,13 @@ struct json_object > > > *util_daxctl_dev_to_json(struct daxctl_dev *dev, > > > if (jobj) > > > json_object_object_add(jdev, "size", jobj); > > > > > > + node = daxctl_dev_get_target_node(dev); > > > + if (node >= 0) { > > > + jobj = json_object_new_int(node); > > > + if (jobj) > > > + json_object_object_add(jdev, "target_node", > > > jobj); > > > + } > > > + > > > > We moved 'numa_node' to the UTIL_JSON_VERBOSE set on "ndctl list" > > should do the same for target node? > > Hm, true. Arguably, the target_node is much more pertinent in system-ram > mode, and /should/ be in the default verbosity? > > One option could be to make it always show if the mode is system-ram, > but not otherwise - but I don't know if that would cause more confusion > as an attribute might seem to magically appear or disappear with the > same command options.. > > Yet another option is, the output right after daxctl-reconfigure-device > always sets UTIL_JSON_VERBOSE, but for daxctl-list, it is only done if > the user supplies it. > > Any preferences on which way to go? 'verbose after reconfigure' sounds good, but you're right, it's more pertinent in the device-dax case, especially when that might be one of our only identification mechanisms for non-NFIT device-dax. So I'm ok to keep it as is on second thought. > > Thanks, > -Vishal
diff --git a/util/json.c b/util/json.c index babdc8c..f521337 100644 --- a/util/json.c +++ b/util/json.c @@ -271,6 +271,7 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev, { const char *devname = daxctl_dev_get_devname(dev); struct json_object *jdev, *jobj; + int node; jdev = json_object_new_object(); if (!devname || !jdev) @@ -284,6 +285,13 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev, if (jobj) json_object_object_add(jdev, "size", jobj); + node = daxctl_dev_get_target_node(dev); + if (node >= 0) { + jobj = json_object_new_int(node); + if (jobj) + json_object_object_add(jdev, "target_node", jobj); + } + return jdev; }
The kernel provides a 'target_node' attribute for dax devices. When converting a dax device to the system-ram mode, the memory is hotplugged into this numa node. It would be helpful to print this in device listings so that it is easy for applications to detect the node to which the new memory belongs. Cc: Dan Williams <dan.j.williams@intel.com> Cc: Dave Hansen <dave.hansen@linux.intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- util/json.c | 8 ++++++++ 1 file changed, 8 insertions(+)