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

[Jonathan Cameron <jonathan.cameron@huawei.com>]

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);
>  }