Re: [PATCH V3 05/21] dax: Add dax_set_ops() for setting dax_operations at bind time

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

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

> From: John Groves <John@Groves.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@groves.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,
>  };