Re: [ndctl PATCH v4 02/10] libdaxctl: cache 'subsystem' in daxctl_ctx

["Verma, Vishal L" <vishal.l.verma@intel.com>]

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(&region_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;



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm