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

["Koralahalli Channabasappa, Smita" <skoralah@amd.com>]

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