Re: [PATCH] dax/hmem, cxl: Defer and resolve ownership of Soft Reserved memory ranges

Koralahalli Channabasappa, Smita
2026-02-17T22:22:00Z

On 2/13/2026 6:47 AM, Gregory Price wrote:
> On Tue, Feb 10, 2026 at 06:45:00AM +0000, Smita Koralahalli wrote:
>
> This is a review generated by kreview-0811365ff2
> Reference: https://github.com/masoncl/review-prompts/
>
> This is not an automated email, I thought this looked valid, the rest of
> the text here is auto-generated.
>
> ~Gregory
>
> ---
>
> [...]
>
>> +static void process_defer_work(void *data)
>> +{
>> + struct platform_device *pdev = data;
>> + int rc;
>> +
>> + /* relies on cxl_acpi and cxl_pci having had a chance to load */
>> + wait_for_device_probe();
>
> [...]
>
>> +static void kill_defer_work(void *data)
>> +{
>> + struct platform_device *pdev = data;
>> +
>> + dax_hmem_flush_work();
>> + dax_hmem_unregister_work(process_defer_work, pdev);
>> +}
>> +
>> static int dax_hmem_platform_probe(struct platform_device *pdev)
>> {
>> + int rc;
>> +
>> + rc = dax_hmem_register_work(process_defer_work, pdev);
>> + if (rc)
>> + return rc;
>> +
>> + rc = devm_add_action_or_reset(&pdev->dev, kill_defer_work, pdev);
>> + if (rc)
>> + return rc;
>> +
>> return walk_hmem_resources(&pdev->dev, hmem_register_device);
>> }
>
> Is there a potential deadlock in the error path here? If
> walk_hmem_resources() encounters a CXL-intersecting range first
> (calling dax_hmem_queue_work(), which schedules process_defer_work
> on system_long_wq), and then a subsequent non-CXL Soft Reserved
> range fails to register, the probe returns an error. The devres
> cleanup then calls kill_defer_work -> dax_hmem_flush_work() ->
> flush_work(&dax_hmem_work).
>
> Meanwhile, process_defer_work calls wait_for_device_probe(), which
> waits for probe_count to reach zero. Since devres_release_all runs
> inside really_probe before driver_probe_device decrements
> probe_count, this looks like a circular wait:
>
> driver_probe_device
> atomic_inc(&probe_count)
> __driver_probe_device
> really_probe
> dax_hmem_platform_probe
> walk_hmem_resources -> hmem_register_device
> CXL range: dax_hmem_queue_work()
> non-CXL range: fails
> returns error
> devres_release_all
> kill_defer_work
> dax_hmem_flush_work
> flush_work(&dax_hmem_work) <-- waits for process_defer_work
> process_defer_work
> wait_for_device_probe() <-- waits for probe_count == 0
> atomic_dec(&probe_count) <-- never reached
>
> The trigger requires both CXL-intersecting and non-CXL Soft Reserved
> ranges with the non-CXL registration failing, so the window is narrow,
> but the deadlock would be permanent if hit.
>
> Would it be safer to cancel the work instead of flushing it in the
> error path, or to avoid queuing deferred work during the initial
> walk?

Yes, you're right, that's a real deadlock. Thanks for pointing this out.

This might affect any wait on the work — flush_work, cancel_work_sync,
all the same problem.

Just using dax_hmem_unregister_work (which only grabs the mutex and
NULLs pointers) avoids the deadlock but opens a use-after-free: the
worker may have already copied fn/data and be mid-execution of
process_defer_work(pdev) while devres tears down the pdev underneath it.

Your suggestion of avoiding queuing during the initial walk is the right
approach I think. Set a flag when hmem_register_device defers a
CXL-intersecting range, queue the work after walk_hmem_resources returns
successfully. Something like:

static int dax_hmem_platform_probe(struct platform_device *pdev)
{
..
..
- return walk_hmem_resources(&pdev->dev, hmem_register_device);
+ rc = walk_hmem_resources(&pdev->dev, hmem_register_device);
+ if (rc)
+ return rc;

+ if (defer) //defer is set under DAX_CXL_MODE_DEFER
+ dax_hmem_queue_work();
}

I think this works but there might be subtleties I'm missing. What do
you think?

Thanks
Smita