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

[Dan Williams <dan.j.williams@intel.com>]

Koralahalli Channabasappa, Smita wrote:
[..]
> +static int hmem_register_cxl_device(..)
> +{
> +	if (region_intersects(..IORES_CXL..) == REGION_DISJOINT)
> +		return 0;
> +
> +	if (cxl_region_contains_soft_reserve(res))
> +		return 0;
> +
> +	return hmem_register_device(host, target_nid, res);
> +}
> 
> +static void process_defer_work(struct work_struct *work)
> +{

I think this also needs:

	guard(device)(&hmem_pdev->dev);
	if (!hmem_pdev->dev.driver)
		return;

...because you can remove the driver while the work is pending.

> +	wait_for_device_probe();
> +	/* Flag lives in device.c */
> +	dax_hmem_initial_probe_done = true;
> +	walk_hmem_resources(&hmem_pdev->dev, hmem_register_cxl_device);

Even though nothing deletes the hmem_pdev device today, I would still
keep its refcount elevated while work is pending.

> +}
> 
> static int dax_hmem_platform_probe(struct platform_device *pdev)
> {
> +	if (work_pending(&dax_hmem_work))
> +		return -EBUSY;
> 
> +	hmem_pdev = pdev;

This wants to be initialized when @dax_hmem_work is initialized.

Otherwise this makes it look like @hmem_pdev can change dynamically. It
is a singleton.

[..]
> A few things I want to confirm:
> 
> 1. Patch 7 (bus.c helpers) drops entirely — no register/unregister API, 
> no mutex, no typedef. Everything lives in hmem.c.
> 
> 2. enum dax_cxl_mode drops — replaced by the single bool 
> dax_hmem_initial_probe_done in device.c.
> 
> 3. dax_hmem_flush_work() exported from dax_hmem.ko so cxl.c gets the 
> module dependency for requirement 2.

Looks good to me.