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

[Dave Jiang <dave.jiang@intel.com>]

On 2/19/26 5:02 PM, Koralahalli Channabasappa, Smita wrote:
> 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.

Ok I missed the history on the single issue work item. Yes what you proposed above should work if it's single issue. and if we are only sending 1 item, a statically declared work context should be sufficient I think. 

DJ

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