On 26/01/08 12:34PM, Jonathan Cameron wrote:
> On Wed,  7 Jan 2026 09:33:16 -0600
> John Groves <[email protected]> wrote:
> 
> > From: John Groves <[email protected]>
> > 
> > Add custom bind/unbind sysfs attributes for the dax bus that check
> > whether a filesystem has registered as a holder (via fs_dax_get())
> > before allowing driver unbind.
> > 
> > When a filesystem like famfs mounts on a dax device, it registers
> > itself as the holder via dax_holder_ops. Previously, there was no
> > mechanism to prevent driver unbind while the filesystem was mounted,
> > which could cause some havoc.
> > 
> > The new unbind_store() checks dax_holder() and returns -EBUSY if
> > a holder is registered, giving userspace proper feedback that the
> > device is in use.
> > 
> > To use our custom bind/unbind handlers instead of the default ones,
> > set suppress_bind_attrs=true on all dax drivers during registration.
> 
> Whilst I appreciate that it is painful, so are many other driver unbinds
> where services are provided to another driver.  Is there any precedence
> for doing something like this? If not, I'd like to see a review on this
> from one of the driver core folk. Maybe Greg KH.
> 
> Might just be a case of calling it something else to avoid userspace
> tooling getting a surprise.

I'll do more digging to see if there are other patterns; feedback/ideas
requested...

> 
> > 
> > Signed-off-by: John Groves <[email protected]>
> > ---
> >  drivers/dax/bus.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> > 
> > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> > index 6e0e28116edc..ed453442739d 100644
> > --- a/drivers/dax/bus.c
> > +++ b/drivers/dax/bus.c
> > @@ -151,9 +151,61 @@ static ssize_t remove_id_store(struct device_driver 
> > *drv, const char *buf,
> >  }
> >  static DRIVER_ATTR_WO(remove_id);
> >  
> > +static const struct bus_type dax_bus_type;
> > +
> > +/*
> > + * Custom bind/unbind handlers for dax bus.
> > + * The unbind handler checks if a filesystem holds the dax device and
> > + * returns -EBUSY if so, preventing driver unbind while in use.
> > + */
> > +static ssize_t unbind_store(struct device_driver *drv, const char *buf,
> > +           size_t count)
> > +{
> > +   struct device *dev;
> > +   int rc = -ENODEV;
> > +
> > +   dev = bus_find_device_by_name(&dax_bus_type, NULL, buf);
> 
>       struct device *dev __free(put_device) = bus_find_device_by_name()...
> 
> and you can just return on error.
> 
> > +   if (dev && dev->driver == drv) {
> With the __free I'd flip this
>       if (!dev || !dev->driver == drv)
>               return -ENODEV;
> 
>       ...
> 

I like it; done.

> > +           struct dev_dax *dev_dax = to_dev_dax(dev);
> > +
> > +           if (dax_holder(dev_dax->dax_dev)) {
> > +                   dev_dbg(dev,
> > +                           "%s: blocking unbind due to active holder\n",
> > +                           __func__);
> > +                   rc = -EBUSY;
> > +                   goto out;
> > +           }
> > +           device_release_driver(dev);
> > +           rc = count;
> > +   }
> > +out:
> > +   put_device(dev);
> > +   return rc;
> > +}
> > +static DRIVER_ATTR_WO(unbind);
> > +
> > +static ssize_t bind_store(struct device_driver *drv, const char *buf,
> > +           size_t count)
> > +{
> > +   struct device *dev;
> > +   int rc = -ENODEV;
> > +
> > +   dev = bus_find_device_by_name(&dax_bus_type, NULL, buf);
> Use __free magic here as well..
> > +   if (dev) {
> > +           rc = device_driver_attach(drv, dev);
> > +           if (!rc)
> > +                   rc = count;
> then this can be
>               if (rc)
>                       return rc;
>               return count;
> 

Done

Thanks!
John


Reply via email to