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

Koralahalli Channabasappa, Smita
2026-02-20T00:02:34Z

Hi Dave,

On 2/18/2026 9:52 AM, Dave Jiang wrote:
>
>
> On 2/9/26 11:44 PM, Smita Koralahalli 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>
>> ---
>> 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);
>> +}
>
> Instead of having a global lock and dealing with all the global variables, why not just do this with the typical work_struct usage pattern and allocate a work item when queuing work?
>
> DJ

Thanks for the feedback.

Just to clarify, are you hinting towards a statically allocated struct
with an embedded work_struct, something like below? Rather than the
typical kmalloc + container_of pattern?

+struct dax_hmem_deferred_ctx {
+ struct work_struct work;
+ dax_hmem_deferred_fn fn;
+ void *data;
+};

+static struct dax_hmem_deferred_ctx dax_hmem_ctx;

+int dax_hmem_register_work(dax_hmem_deferred_fn fn, void *data)
+{
+ if (dax_hmem_ctx.fn)
+ return -EINVAL;

+ INIT_WORK(&dax_hmem_ctx.work, hmem_deferred_work);
..

My understanding is that Dan wanted this to remain a singleton deferred
work item queued once and flushed from dax_cxl. I think with kmalloc +
container_of approach, every call would allocate and queue a new
independent work item..

Regarding the mutex: looking at it again, it may not be necessary I
think. If we can rely on the call ordering (register_work() before
queue_work()), and if flush_work() in kill_defer_work() ensures the work
has fully completed before unregister_work() NULLs the pointers, then
the static struct above would be sufficient without additional locking.
If I'm missing a scenario or race here, please correct me.

Thanks,
Smita

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