| Message ID | 20190528222440.30392-3-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | daxctl: add a new reconfigure-device command | expand |
On Tue, May 28, 2019 at 3:24 PM Vishal Verma <vishal.l.verma@intel.com> wrote: > > The 'DAX subsystem' in effect is determined at region or device init > time, and dictates the sysfs base paths for all device/region > operations. In preparation for adding bind/unbind functionality, cache > the subsystem as determined at init time in the library context. I'm missing how this patch determines the subsystem at init time? ...more below. > > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > daxctl/lib/libdaxctl.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c > index 70f896b..f8f5b8c 100644 > --- a/daxctl/lib/libdaxctl.c > +++ b/daxctl/lib/libdaxctl.c > @@ -46,6 +46,7 @@ struct daxctl_ctx { > void *userdata; > int regions_init; > struct list_head regions; > + enum dax_subsystem subsys; > }; > > /** > @@ -96,6 +97,7 @@ DAXCTL_EXPORT int daxctl_new(struct daxctl_ctx **ctx) > dbg(c, "log_priority=%d\n", c->ctx.log_priority); > *ctx = c; > list_head_init(&c->regions); > + c->subsys = DAX_UNKNOWN; > > return 0; > } > @@ -454,14 +456,18 @@ static void dax_devices_init(struct daxctl_region *region) > for (i = 0; i < ARRAY_SIZE(dax_subsystems); i++) { > char *region_path; > > - if (i == DAX_BUS) > + if (i == DAX_BUS) { > region_path = region->region_path; > - else if (i == DAX_CLASS) { > + if (ctx->subsys == DAX_UNKNOWN) > + ctx->subsys = DAX_BUS; > + } else if (i == DAX_CLASS) { > if (asprintf(®ion_path, "%s/dax", > region->region_path) < 0) { > dbg(ctx, "region path alloc fail\n"); > continue; > } > + if (ctx->subsys == DAX_UNKNOWN) > + ctx->subsys = DAX_CLASS; > } else > continue; > sysfs_device_parse(ctx, region_path, daxdev_fmt, region, dax_devices_init() is just blindly looping through both device models attempting to add devices. If this patch was detecting device-models I would expect it would be looking for the first successful sysfs_device_parse() to judge which of those blind shots actually worked.
On Tue, 2019-05-28 at 17:27 -0700, Dan Williams wrote: > On Tue, May 28, 2019 at 3:24 PM Vishal Verma <vishal.l.verma@intel.com> wrote: > > The 'DAX subsystem' in effect is determined at region or device init > > time, and dictates the sysfs base paths for all device/region > > operations. In preparation for adding bind/unbind functionality, cache > > the subsystem as determined at init time in the library context. > > I'm missing how this patch determines the subsystem at init time? ...more below. > [..] > > } > > @@ -454,14 +456,18 @@ static void dax_devices_init(struct daxctl_region *region) > > for (i = 0; i < ARRAY_SIZE(dax_subsystems); i++) { > > char *region_path; > > > > - if (i == DAX_BUS) > > + if (i == DAX_BUS) { > > region_path = region->region_path; > > - else if (i == DAX_CLASS) { > > + if (ctx->subsys == DAX_UNKNOWN) > > + ctx->subsys = DAX_BUS; > > + } else if (i == DAX_CLASS) { > > if (asprintf(®ion_path, "%s/dax", > > region->region_path) < 0) { > > dbg(ctx, "region path alloc fail\n"); > > continue; > > } > > + if (ctx->subsys == DAX_UNKNOWN) > > + ctx->subsys = DAX_CLASS; > > } else > > continue; > > sysfs_device_parse(ctx, region_path, daxdev_fmt, region, > > dax_devices_init() is just blindly looping through both device models > attempting to add devices. If this patch was detecting device-models I > would expect it would be looking for the first successful > sysfs_device_parse() to judge which of those blind shots actually > worked. I see - I was definitely misunderstanding how this worked. I'll fix up for v5, essentially something like: - sysfs_device_parse(ctx, region_path, daxdev_fmt, region, - add_dax_dev); + if (sysfs_device_parse(ctx, region_path, daxdev_fmt, region, + add_dax_dev) == 0) + ctx->subsys = i;
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c index 70f896b..f8f5b8c 100644 --- a/daxctl/lib/libdaxctl.c +++ b/daxctl/lib/libdaxctl.c @@ -46,6 +46,7 @@ struct daxctl_ctx { void *userdata; int regions_init; struct list_head regions; + enum dax_subsystem subsys; }; /** @@ -96,6 +97,7 @@ DAXCTL_EXPORT int daxctl_new(struct daxctl_ctx **ctx) dbg(c, "log_priority=%d\n", c->ctx.log_priority); *ctx = c; list_head_init(&c->regions); + c->subsys = DAX_UNKNOWN; return 0; } @@ -454,14 +456,18 @@ static void dax_devices_init(struct daxctl_region *region) for (i = 0; i < ARRAY_SIZE(dax_subsystems); i++) { char *region_path; - if (i == DAX_BUS) + if (i == DAX_BUS) { region_path = region->region_path; - else if (i == DAX_CLASS) { + if (ctx->subsys == DAX_UNKNOWN) + ctx->subsys = DAX_BUS; + } else if (i == DAX_CLASS) { if (asprintf(®ion_path, "%s/dax", region->region_path) < 0) { dbg(ctx, "region path alloc fail\n"); continue; } + if (ctx->subsys == DAX_UNKNOWN) + ctx->subsys = DAX_CLASS; } else continue; sysfs_device_parse(ctx, region_path, daxdev_fmt, region, @@ -539,6 +545,8 @@ static void __dax_regions_init(struct daxctl_ctx *ctx, enum dax_subsystem subsys free(dev_path); if (!region) err(ctx, "add_dax_region() for %s failed\n", de->d_name); + if (ctx->subsys == DAX_UNKNOWN) + ctx->subsys = subsys; } closedir(dir); }
The 'DAX subsystem' in effect is determined at region or device init time, and dictates the sysfs base paths for all device/region operations. In preparation for adding bind/unbind functionality, cache the subsystem as determined at init time in the library context. Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- daxctl/lib/libdaxctl.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)