Re: [PATCH v5 3/7] cxl/acpi: Add background worker to coordinate with cxl_mem probe completion

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

Koralahalli Channabasappa, Smita wrote:
[..]
> >   static int hmem_register_device(struct device *host, int target_nid,
> >   				const struct resource *res)
> >   {
> > +	struct dax_defer_work *work = dev_get_drvdata(host);
> >   	struct platform_device *pdev;
> >   	struct memregion_info info;
> >   	long id;
> > @@ -70,14 +106,21 @@ static int hmem_register_device(struct device *host, int target_nid,
> >   	if (IS_ENABLED(CONFIG_CXL_REGION) &&
> >   	    region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
> >   			      IORES_DESC_CXL) != REGION_DISJOINT) {
> 
> I may be wrong here, but could this check fail?

It can fail, but for the case where ACPI0017 is present and CXL windows
exist, the failure cases would only be the extreme ones like OOM killer.

> While request_module() ensures that cxl_acpi and cxl_pci are requested
> for loading, it does not guarantee that either has completed
> initialization or that region enumeration (i.e add_cxl_resources())
> has finished by the time we reach this check.

No, outside of someone doing something silly like passing
"driver_async_probe=cxl_acpi" on the kernel command line then
request_module() will complete synchronously (btw, should close that
possibility off with PROBE_FORCE_SYNCHRONOUS).

When request_module() returns module_init() for the requested module
will have completed. ACPI devices will have been enumerated by this
point, so cxl_acpi_probe() will have also run by the time module_init()
completes.

> We also haven't called wait_for_device_probe() at this point, which is 
> typically used to block until all pending device probes are complete.

wait_for_device_probe() is only needed for async probing, deferred
probing, and dependent device probing. cxl_acpi is none of those cases.
ACPI devices are always enumerated before userspace is up, so the
initial driver attach can always assume to have completed in module_init
context.

wait_for_device_probe() is needed for cxl_pci attach because cxl_pci
attach is async and it creates dependent devices that fire off their own
module requests.

As I noted in the changelog MODULE_SOFTDEP() is not reliable for
ordering, but request_module() is reliable for ordering. We could go so
far as to have symbol dependencies to require module loading to succeed,
but I don't think that is needed here.

See that approach in the for-6.18/cxl-probe-order RFC branch for cxl_mem
and cxl_port:

https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/log/?h=for-6.18/cxl-probe-order