[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260108123450.00004eac@huawei.com>
Date: Thu, 8 Jan 2026 12:34:50 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: John Groves <John@...ves.net>
CC: Miklos Szeredi <miklos@...redi.hu>, Dan Williams
<dan.j.williams@...el.com>, Bernd Schubert <bschubert@....com>, "Alison
Schofield" <alison.schofield@...el.com>, John Groves <jgroves@...ron.com>,
Jonathan Corbet <corbet@....net>, Vishal Verma <vishal.l.verma@...el.com>,
Dave Jiang <dave.jiang@...el.com>, Matthew Wilcox <willy@...radead.org>, Jan
Kara <jack@...e.cz>, Alexander Viro <viro@...iv.linux.org.uk>, "David
Hildenbrand" <david@...nel.org>, Christian Brauner <brauner@...nel.org>,
"Darrick J . Wong" <djwong@...nel.org>, Randy Dunlap <rdunlap@...radead.org>,
Jeff Layton <jlayton@...nel.org>, Amir Goldstein <amir73il@...il.com>, Stefan
Hajnoczi <shajnocz@...hat.com>, Joanne Koong <joannelkoong@...il.com>, Josef
Bacik <josef@...icpanda.com>, Bagas Sanjaya <bagasdotme@...il.com>, Chen
Linxuan <chenlinxuan@...ontech.com>, "James Morse" <james.morse@....com>,
Fuad Tabba <tabba@...gle.com>, "Sean Christopherson" <seanjc@...gle.com>,
Shivank Garg <shivankg@....com>, Ackerley Tng <ackerleytng@...gle.com>,
Gregory Price <gourry@...rry.net>, Aravind Ramesh <arramesh@...ron.com>, Ajay
Joshi <ajayjoshi@...ron.com>, <venkataravis@...ron.com>,
<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<nvdimm@...ts.linux.dev>, <linux-cxl@...r.kernel.org>,
<linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH V3 07/21] dax: prevent driver unbind while filesystem
holds device
On Wed, 7 Jan 2026 09:33:16 -0600
John Groves <John@...ves.net> wrote:
> From: John Groves <John@...ves.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@...ves.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);
> }
Powered by blists - more mailing lists