Re: [ndctl PATCH 04/10] libdaxctl: add an API to determine if memory is movable
On Fri, Oct 18, 2019 at 12:57 PM Verma, Vishal L
wrote:
>
>
> On Fri, 2019-10-18 at 11:54 -0700, Dan Williams wrote:
> > On Wed, Oct 2, 2019 at 4:49 PM Vishal Verma
> > wrote:
> > > By default, daxctl always attempts to online new memory sections as
> > > 'movable' so that routine kernel allocations aren't serviced from this
> > > memory, and the memory is later removable via hot-unplug.
> > >
> > > System configuration, or other agents (such as udev rules) may race
> > > 'daxctl' to online memory, and this may result in the memory not being
> > > 'movable'. Add an interface to query the movability of a memory object
> > > associated with a dax device.
> > >
> > > This is in preparation to both display a 'movable' attribute in device
> > > listings, as well as optionally allowing memory to be onlined as
> > > non-movable.
> > >
> > > Cc: Dan Williams
> > > Reported-by: Ben Olson
> > > Signed-off-by: Vishal Verma
> > > ---
> > > daxctl/lib/libdaxctl-private.h | 20 +
> > > daxctl/lib/libdaxctl.c | 77 --
> > > daxctl/lib/libdaxctl.sym | 5 +++
> > > daxctl/libdaxctl.h | 1 +
> > > 4 files changed, 100 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/daxctl/lib/libdaxctl-private.h
> > > b/daxctl/lib/libdaxctl-private.h
> > > index 7ba3c46..82939bb 100644
> > > --- a/daxctl/lib/libdaxctl-private.h
> > > +++ b/daxctl/lib/libdaxctl-private.h
> > > @@ -44,6 +44,25 @@ enum memory_op {
> > > MEM_SET_ONLINE,
> > > MEM_IS_ONLINE,
> > > MEM_COUNT,
> > > + MEM_FIND_ZONE,
> >
> > This is private so the naming is not too big a concern, but isn't this
> > a MEM_GET_ZONE? A find operation to me is something that can succeed
> > to find nothing, whereas a get operation fail assumes the association
> > can always be retrieved barring exceptional conditions.
>
> Hm, my personal view of find vs. get was that 'get' grabs a reference
> (or similar) to something which we know how to get to (have a pointer
> directly) etc.
Oh, that's get as in get/put. I'm talking about get/set. Where the
zone is a property memblock that can set and retrieved.
> 'Find' is more of a 'go searching for' something - and it
> may involve walking and looping over data structures.
>
> But I'm not too opposed to this, and can change to 'get' if that follows
> convention better.
Like I said it's private, so it's not a big deal (i.e. API users won't
see it), but ndctl does have a few apis that walk and loop over data
structures using a get verb, or the ones that retrieve the dynamic
state of a sysfs attribute.
I will grant you that in hindsight some of the "get_by" apis might
have been better named "find".
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [ndctl PATCH 04/10] libdaxctl: add an API to determine if memory is movable
On Wed, Oct 2, 2019 at 4:49 PM Vishal Verma wrote:
>
> By default, daxctl always attempts to online new memory sections as
> 'movable' so that routine kernel allocations aren't serviced from this
> memory, and the memory is later removable via hot-unplug.
>
> System configuration, or other agents (such as udev rules) may race
> 'daxctl' to online memory, and this may result in the memory not being
> 'movable'. Add an interface to query the movability of a memory object
> associated with a dax device.
>
> This is in preparation to both display a 'movable' attribute in device
> listings, as well as optionally allowing memory to be onlined as
> non-movable.
>
> Cc: Dan Williams
> Reported-by: Ben Olson
> Signed-off-by: Vishal Verma
> ---
> daxctl/lib/libdaxctl-private.h | 20 +
> daxctl/lib/libdaxctl.c | 77 --
> daxctl/lib/libdaxctl.sym | 5 +++
> daxctl/libdaxctl.h | 1 +
> 4 files changed, 100 insertions(+), 3 deletions(-)
>
> diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
> index 7ba3c46..82939bb 100644
> --- a/daxctl/lib/libdaxctl-private.h
> +++ b/daxctl/lib/libdaxctl-private.h
> @@ -44,6 +44,25 @@ enum memory_op {
> MEM_SET_ONLINE,
> MEM_IS_ONLINE,
> MEM_COUNT,
> + MEM_FIND_ZONE,
This is private so the naming is not too big a concern, but isn't this
a MEM_GET_ZONE? A find operation to me is something that can succeed
to find nothing, whereas a get operation fail assumes the association
can always be retrieved barring exceptional conditions.
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
[ndctl PATCH 04/10] libdaxctl: add an API to determine if memory is movable
By default, daxctl always attempts to online new memory sections as
'movable' so that routine kernel allocations aren't serviced from this
memory, and the memory is later removable via hot-unplug.
System configuration, or other agents (such as udev rules) may race
'daxctl' to online memory, and this may result in the memory not being
'movable'. Add an interface to query the movability of a memory object
associated with a dax device.
This is in preparation to both display a 'movable' attribute in device
listings, as well as optionally allowing memory to be onlined as
non-movable.
Cc: Dan Williams
Reported-by: Ben Olson
Signed-off-by: Vishal Verma
---
daxctl/lib/libdaxctl-private.h | 20 +
daxctl/lib/libdaxctl.c | 77 --
daxctl/lib/libdaxctl.sym | 5 +++
daxctl/libdaxctl.h | 1 +
4 files changed, 100 insertions(+), 3 deletions(-)
diff --git a/daxctl/lib/libdaxctl-private.h b/daxctl/lib/libdaxctl-private.h
index 7ba3c46..82939bb 100644
--- a/daxctl/lib/libdaxctl-private.h
+++ b/daxctl/lib/libdaxctl-private.h
@@ -44,6 +44,25 @@ enum memory_op {
MEM_SET_ONLINE,
MEM_IS_ONLINE,
MEM_COUNT,
+ MEM_FIND_ZONE,
+};
+
+/* OR-able flags, 1, 2, 4, 8 etc */
+enum memory_op_status {
+ MEM_ST_OK = 0,
+ MEM_ST_ZONE_INCONSISTENT = 1,
+};
+
+enum memory_zones {
+ MEM_ZONE_UNKNOWN = 1,
+ MEM_ZONE_MOVABLE,
+ MEM_ZONE_NORMAL,
+};
+
+static const char *zone_strings[] = {
+ [MEM_ZONE_UNKNOWN] = "mixed",
+ [MEM_ZONE_NORMAL] = "Normal",
+ [MEM_ZONE_MOVABLE] = "Movable",
};
/**
@@ -86,6 +105,7 @@ struct daxctl_memory {
size_t buf_len;
char *node_path;
unsigned long block_size;
+ enum memory_zones zone;
};
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 6243857..4460174 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -1159,6 +1159,58 @@ static int offline_one_memblock(struct daxctl_memory
*mem, char *memblock)
return rc;
}
+static int memblock_find_zone(struct daxctl_memory *mem, char *memblock,
+ int *status)
+{
+ struct daxctl_dev *dev = daxctl_memory_get_dev(mem);
+ const char *devname = daxctl_dev_get_devname(dev);
+ struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+ enum memory_zones cur_zone;
+ int len = mem->buf_len, rc;
+ char buf[SYSFS_ATTR_SIZE];
+ char *path = mem->mem_buf;
+ const char *node_path;
+
+ rc = memblock_is_online(mem, memblock);
+ if (rc < 0)
+ return rc;
+ if (rc == 0)
+ return -ENXIO;
+
+ node_path = daxctl_memory_get_node_path(mem);
+ if (!node_path)
+ return -ENXIO;
+
+ rc = snprintf(path, len, "%s/%s/valid_zones", node_path, memblock);
+ if (rc < 0)
+ return -ENOMEM;
+
+ rc = sysfs_read_attr(ctx, path, buf);
+ if (rc) {
+ err(ctx, "%s: Failed to read %s: %s\n",
+ devname, path, strerror(-rc));
+ return rc;
+ }
+
+ if (strcmp(buf, zone_strings[MEM_ZONE_MOVABLE]) == 0)
+ cur_zone = MEM_ZONE_MOVABLE;
+ else if (strcmp(buf, zone_strings[MEM_ZONE_NORMAL]) == 0)
+ cur_zone = MEM_ZONE_NORMAL;
+ else
+ cur_zone = MEM_ZONE_UNKNOWN;
+
+ if (mem->zone) {
+ if (mem->zone == cur_zone)
+ return 0;
+ else
+ *status |= MEM_ST_ZONE_INCONSISTENT;
+ } else {
+ mem->zone = cur_zone;
+ }
+
+ return 0;
+}
+
static bool memblock_in_dev(struct daxctl_memory *mem, const char *memblock)
{
const char *mem_base = "/sys/devices/system/memory/";
@@ -1211,7 +1263,7 @@ static bool memblock_in_dev(struct daxctl_memory *mem,
const char *memblock)
}
static int op_for_one_memblock(struct daxctl_memory *mem, char *memblock,
- enum memory_op op)
+ enum memory_op op, int *status)
{
struct daxctl_dev *dev = daxctl_memory_get_dev(mem);
const char *devname = daxctl_dev_get_devname(dev);
@@ -1234,6 +1286,8 @@ static int op_for_one_memblock(struct daxctl_memory *mem,
char *memblock,
return !rc;
case MEM_COUNT:
return 0;
+ case MEM_FIND_ZONE:
+ return memblock_find_zone(mem, memblock, status);
}
err(ctx, "%s: BUG: unknown op: %d\n", devname, op);
@@ -1245,8 +1299,8 @@ static int daxctl_memory_op(struct daxctl_memory *mem,
enum memory_op op)
struct daxctl_dev *dev = daxctl_memory_get_dev(mem);
const char *devname = daxctl_dev_get_devname(dev);
struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+ int rc, count = 0, status_flags = 0;
const char *node_path;
- int rc, count = 0;
struct dirent *de;
DIR *node_dir;
@@ -1265,7
