[PATCH V3 07/21] dax: prevent driver unbind while filesystem holds device

John Groves posted 21 patches 1 month ago
[PATCH V3 07/21] dax: prevent driver unbind while filesystem holds device
Posted by John Groves 1 month ago
From: John Groves <John@Groves.net>

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.

Signed-off-by: John Groves <john@groves.net>
---
 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);
+	if (dev && dev->driver == drv) {
+		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);
+	if (dev) {
+		rc = device_driver_attach(drv, dev);
+		if (!rc)
+			rc = count;
+	}
+	put_device(dev);
+	return rc;
+}
+static DRIVER_ATTR_WO(bind);
+
 static struct attribute *dax_drv_attrs[] = {
 	&driver_attr_new_id.attr,
 	&driver_attr_remove_id.attr,
+	&driver_attr_bind.attr,
+	&driver_attr_unbind.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(dax_drv);
@@ -1591,6 +1643,7 @@ int __dax_driver_register(struct dax_device_driver *dax_drv,
 	drv->name = mod_name;
 	drv->mod_name = mod_name;
 	drv->bus = &dax_bus_type;
+	drv->suppress_bind_attrs = true;
 
 	return driver_register(drv);
 }
-- 
2.49.0
Re: [PATCH V3 07/21] dax: prevent driver unbind while filesystem holds device
Posted by John Groves 3 weeks, 6 days ago
On 26/01/07 09:33AM, John Groves wrote:
> From: John Groves <John@Groves.net>
> 
> 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.
> 
> Signed-off-by: John Groves <john@groves.net>

After a discussion with Dan Williams, I will be dropping this patch
from the series. If the fsdev-mode driver gets unbound under famfs,
famfs will just stop working.

Based on feedback so far, V4 should be coming in the next few days.

Regards,
John
Re: [PATCH V3 07/21] dax: prevent driver unbind while filesystem holds device
Posted by Jonathan Cameron 1 month ago
On Wed,  7 Jan 2026 09:33:16 -0600
John Groves <John@Groves.net> wrote:

> From: John Groves <John@Groves.net>
> 
> 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.

> 
> Signed-off-by: John Groves <john@groves.net>
> ---
>  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;

	...

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

> +	}
> +	put_device(dev);
> +	return rc;
> +}
> +static DRIVER_ATTR_WO(bind);
> +
>  static struct attribute *dax_drv_attrs[] = {
>  	&driver_attr_new_id.attr,
>  	&driver_attr_remove_id.attr,
> +	&driver_attr_bind.attr,
> +	&driver_attr_unbind.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(dax_drv);
> @@ -1591,6 +1643,7 @@ int __dax_driver_register(struct dax_device_driver *dax_drv,
>  	drv->name = mod_name;
>  	drv->mod_name = mod_name;
>  	drv->bus = &dax_bus_type;
> +	drv->suppress_bind_attrs = true;
>  
>  	return driver_register(drv);
>  }
Re: [PATCH V3 07/21] dax: prevent driver unbind while filesystem holds device
Posted by John Groves 1 month ago
On 26/01/08 12:34PM, Jonathan Cameron wrote:
> On Wed,  7 Jan 2026 09:33:16 -0600
> John Groves <John@Groves.net> wrote:
> 
> > From: John Groves <John@Groves.net>
> > 
> > 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 <john@groves.net>
> > ---
> >  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