[ndctl,v6,05/13] daxctl/list: add target_node for device listings
diff mbox series

Message ID 20190717225400.9494-6-vishal.l.verma@intel.com
State Superseded
Headers show
Series
  • daxctl: add a new reconfigure-device command
Related show

Commit Message

Vishal Verma July 17, 2019, 10:53 p.m. UTC
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(+)

Comments

Dan Williams July 18, 2019, 11:41 p.m. UTC | #1
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?
Vishal Verma July 19, 2019, 6:08 p.m. UTC | #2
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
Dan Williams July 19, 2019, 6:36 p.m. UTC | #3
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

Patch
diff mbox series

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