linux-kernel - Re: [PATCH V3 05/21] dax: Add dax_set_ops() for setting dax

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260108120619.00001bc5@huawei.com>
Date: Thu, 8 Jan 2026 12:06:19 +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 05/21] dax: Add dax_set_ops() for setting
 dax_operations at bind time

On Wed,  7 Jan 2026 09:33:14 -0600
John Groves <John@...ves.net> wrote:

> From: John Groves <John@...ves.net>
> 
> The dax_device is created (in the non-pmem case) at hmem probe time via
> devm_create_dev_dax(), before we know which driver (device_dax,
> fsdev_dax, or kmem) will bind - by calling alloc_dax() with NULL ops,
> drivers (i.e. fsdev_dax) that need specific dax_operations must set
> them later.
> 
> Add dax_set_ops() exported function so fsdev_dax can set its ops at
> probe time and clear them on remove. device_dax doesn't need ops since
> it uses the mmap fault path directly.
> 
> Use cmpxchg() to atomically set ops only if currently NULL, returning
> -EBUSY if ops are already set. This prevents accidental double-binding.
> Clearing ops (NULL) always succeeds.
> 
> Signed-off-by: John Groves <john@...ves.net>
Hi John

This one runs into the fun mess of mixing devm and other calls.
I'd advise you just don't do it because it makes code much harder
to review and hits the 'smells bad' button.

Jonathan

> ---
>  drivers/dax/fsdev.c | 12 ++++++++++++
>  drivers/dax/super.c | 38 +++++++++++++++++++++++++++++++++++++-
>  include/linux/dax.h |  1 +
>  3 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> index 9e2f83aa2584..3f4f593896e3 100644
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
> @@ -330,12 +330,24 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
>  	if (rc)
>  		return rc;
>  
> +	/* Set the dax operations for fs-dax access path */
> +	rc = dax_set_ops(dax_dev, &dev_dax_ops);
> +	if (rc)
> +		return rc;
> +
>  	run_dax(dax_dev);
>  	return devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
>  }
>  
> +static void fsdev_dax_remove(struct dev_dax *dev_dax)
> +{
> +	/* Clear ops on unbind so they aren't used with a different driver */
> +	dax_set_ops(dev_dax->dax_dev, NULL);

Generally orderings of calls that mix devm and stuff done manually in remove are
a bad idea.  They can be safe (and this one probably is) but it adds a review
burden that is best avoided.

Once you stop using devm_ you need to stop it for everything.  So either
use a devm_add_action_or_reset for this or drop the one for fsdev_kill and
call that code here instead.

> +}
> +
>  static struct dax_device_driver fsdev_dax_driver = {
>  	.probe = fsdev_dax_probe,
> +	.remove = fsdev_dax_remove,
>  	.type = DAXDRV_FSDEV_TYPE,
>  };