Re: [PATCH v6 7/9] dax: Add deferred-work helpers for dax_hmem and dax_cxl coordination

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

On Tue, 10 Feb 2026 06:44:59 +0000
Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:

> Add helpers to register, queue and flush the deferred work.
> 
> These helpers allow dax_hmem to execute ownership resolution outside the
> probe context before dax_cxl binds.
> 
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>

The sanity checks on valid inputs to me seem excessive for something
that is intended to have a very narrow usecase. I'm also not sure it's
harmful to just not bother with the parameter checking.

Otherwise seems fine to me.

> ---
>  drivers/dax/bus.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/dax/bus.h |  7 ++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 5f387feb95f0..92b88952ede1 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -25,6 +25,64 @@ DECLARE_RWSEM(dax_region_rwsem);
>   */
>  DECLARE_RWSEM(dax_dev_rwsem);
>  
> +static DEFINE_MUTEX(dax_hmem_lock);
> +static dax_hmem_deferred_fn hmem_deferred_fn;
> +static void *dax_hmem_data;
> +
> +static void hmem_deferred_work(struct work_struct *work)
> +{
> +	dax_hmem_deferred_fn fn;
> +	void *data;
> +
> +	scoped_guard(mutex, &dax_hmem_lock) {
> +		fn = hmem_deferred_fn;
> +		data = dax_hmem_data;
> +	}
> +
> +	if (fn)
> +		fn(data);
> +}
> +
> +static DECLARE_WORK(dax_hmem_work, hmem_deferred_work);
> +
> +int dax_hmem_register_work(dax_hmem_deferred_fn fn, void *data)
> +{
> +	guard(mutex)(&dax_hmem_lock);
> +
> +	if (hmem_deferred_fn)
> +		return -EINVAL;
What happens if we drop the check and therefore need to return int
from these + handle errors?

The worst that happens is hmem_deferred_fn == NULL and we set the
data (might also be NULL, we don't care).
To me that looks harmless.

> +
> +	hmem_deferred_fn = fn;
> +	dax_hmem_data = data;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dax_hmem_register_work);
> +
> +int dax_hmem_unregister_work(dax_hmem_deferred_fn fn, void *data)
> +{
> +	guard(mutex)(&dax_hmem_lock);
> +
> +	if (hmem_deferred_fn != fn || dax_hmem_data != data)
> +		return -EINVAL;

Do we need the sanity check?  I'd just unconditionally clear them
both.

> +
> +	hmem_deferred_fn = NULL;
> +	dax_hmem_data = NULL;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dax_hmem_unregister_work);
> +
> +void dax_hmem_queue_work(void)
> +{
> +	queue_work(system_long_wq, &dax_hmem_work);
> +}
> +EXPORT_SYMBOL_GPL(dax_hmem_queue_work);
> +
> +void dax_hmem_flush_work(void)
> +{
> +	flush_work(&dax_hmem_work);
> +}
> +EXPORT_SYMBOL_GPL(dax_hmem_flush_work);
> +
>  #define DAX_NAME_LEN 30
>  struct dax_id {
>  	struct list_head list;
> diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h
> index cbbf64443098..b58a88e8089c 100644
> --- a/drivers/dax/bus.h
> +++ b/drivers/dax/bus.h
> @@ -41,6 +41,13 @@ struct dax_device_driver {
>  	void (*remove)(struct dev_dax *dev);
>  };
>  
> +typedef void (*dax_hmem_deferred_fn)(void *data);
> +
> +int dax_hmem_register_work(dax_hmem_deferred_fn fn, void *data);
> +int dax_hmem_unregister_work(dax_hmem_deferred_fn fn, void *data);
> +void dax_hmem_queue_work(void);
> +void dax_hmem_flush_work(void);
> +
>  int __dax_driver_register(struct dax_device_driver *dax_drv,
>  		struct module *module, const char *mod_name);
>  #define dax_driver_register(driver) \