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

Jonathan Cameron
2026-03-09T14:49:51Z

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) \