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.
