On Thu, Mar 19, 2026 at 02:29:10PM +0000, Jonathan Cameron wrote:
> On Thu, 19 Mar 2026 01:14:59 +0000
> Smita Koralahalli <[email protected]> wrote:
>
> > The current probe time ownership check for Soft Reserved memory based
> > solely on CXL window intersection is insufficient. dax_hmem probing is not
> > always guaranteed to run after CXL enumeration and region assembly, which
> > can lead to incorrect ownership decisions before the CXL stack has
> > finished publishing windows and assembling committed regions.
> >
> > Introduce deferred ownership handling for Soft Reserved ranges that
> > intersect CXL windows. When such a range is encountered during the
> > initial dax_hmem probe, schedule deferred work to wait for the CXL stack
> > to complete enumeration and region assembly before deciding ownership.
> >
> > Once the deferred work runs, evaluate each Soft Reserved range
> > individually: if a CXL region fully contains the range, skip it and let
> > dax_cxl bind. Otherwise, register it with dax_hmem. This per-range
> > ownership model avoids the need for CXL region teardown and
> > alloc_dax_region() resource exclusion prevents double claiming.
> >
> > Introduce a boolean flag dax_hmem_initial_probe to live inside device.c
> > so it survives module reload. Ensure dax_cxl defers driver registration
> > until dax_hmem has completed ownership resolution. dax_cxl calls
> > dax_hmem_flush_work() before cxl_driver_register(), which both waits for
> > the deferred work to complete and creates a module symbol dependency that
> > forces dax_hmem.ko to load before dax_cxl.
> >
> > Co-developed-by: Dan Williams <[email protected]>
> > Signed-off-by: Dan Williams <[email protected]>
> > Signed-off-by: Smita Koralahalli <[email protected]>
> Hi Smita,
>
> I think this is very likely to be what is causing the bug Alison
> saw in cxl_test.
>
> It looks to be possible to flush work before the work structure has
> been configured. Even though it's not on a work queue and there is
> nothing to do, there are early sanity checks that fail giving the warning
> Alison reported.
>
> A couple of ways to fix that inline. I'd be tempted to both initialize
> the function statically and gate against flushing if the whole thing isn't
> set up yet.
>
> Jonathan
snip
>
> > diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
> > index 1e3424358490..8c574123bd3b 100644
> > --- a/drivers/dax/hmem/hmem.c
> > +++ b/drivers/dax/hmem/hmem.c
> > @@ -3,6 +3,7 @@
> > #include <linux/memregion.h>
> > #include <linux/module.h>
> > #include <linux/dax.h>
> > +#include <cxl/cxl.h>
> > #include "../bus.h"
> >
> > static bool region_idle;
> > @@ -58,6 +59,19 @@ static void release_hmem(void *pdev)
> > platform_device_unregister(pdev);
> > }
> >
> > +struct dax_defer_work {
> > + struct platform_device *pdev;
> > + struct work_struct work;
> > +};
> > +
> > +static struct dax_defer_work dax_hmem_work;
>
> static struct dax_defer_work dax_hmem_work = {
> .work = __WORK_INITIALIZER(&dax_hmem_work.work,
> process_defer_work),
> };
> or something similar.
>
Just confirming this stopped the WARN:
-static struct dax_defer_work dax_hmem_work;
+static void process_defer_work(struct work_struct *work);
+
+static struct dax_defer_work dax_hmem_work = {
+ .work = __WORK_INITIALIZER(dax_hmem_work.work, process_defer_work),
+};
>
> > +
> > +void dax_hmem_flush_work(void)
> > +{
> > + flush_work(&dax_hmem_work.work);
> > +}
> > +EXPORT_SYMBOL_GPL(dax_hmem_flush_work);
> > +
> > static int hmem_register_device(struct device *host, int target_nid,
> > const struct resource *res)
> > {
> > @@ -69,8 +83,11 @@ static int hmem_register_device(struct device *host, int
> > target_nid,
> > if (IS_ENABLED(CONFIG_DEV_DAX_CXL) &&
> > region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
> > IORES_DESC_CXL) != REGION_DISJOINT) {
> > - dev_dbg(host, "deferring range to CXL: %pr\n", res);
> > - return 0;
> > + if (!dax_hmem_initial_probe) {
> > + dev_dbg(host, "deferring range to CXL: %pr\n", res);
> > + queue_work(system_long_wq, &dax_hmem_work.work);
> > + return 0;
> > + }
> > }
> >
> > rc = region_intersects_soft_reserve(res->start, resource_size(res));
> > @@ -123,8 +140,48 @@ static int hmem_register_device(struct device *host,
> > int target_nid,
> > return rc;
> > }
> >
> > +static int hmem_register_cxl_device(struct device *host, int target_nid,
> > + const struct resource *res)
> > +{
> > + if (region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
> > + IORES_DESC_CXL) == REGION_DISJOINT)
> > + return 0;
> > +
> > + if (cxl_region_contains_resource((struct resource *)res)) {
> > + dev_dbg(host, "CXL claims resource, dropping: %pr\n", res);
> > + return 0;
> > + }
> > +
> > + dev_dbg(host, "CXL did not claim resource, registering: %pr\n", res);
> > + return hmem_register_device(host, target_nid, res);
> > +}
> > +
> > +static void process_defer_work(struct work_struct *w)
> > +{
> > + struct dax_defer_work *work = container_of(w, typeof(*work), work);
> > + struct platform_device *pdev = work->pdev;
> If you do the suggested __INITIALIZE_WORK() then I'd add
> a paranoid
>
> if (!work->pdev)
> return;
> We don't actually queue the work before pdev is set, but that might
> be obvious once we spilt up assigning the function and the data
> it uses.
>
> > +
> > + wait_for_device_probe();
> > +
> > + guard(device)(&pdev->dev);
> > + if (!pdev->dev.driver)
> > + return;
> > +
> > + dax_hmem_initial_probe = true;
> > + walk_hmem_resources(&pdev->dev, hmem_register_cxl_device);
> > +}
> > +
> > static int dax_hmem_platform_probe(struct platform_device *pdev)
> > {
> > + if (work_pending(&dax_hmem_work.work))
> > + return -EBUSY;
> > +
> > + if (!dax_hmem_work.pdev) {
> > + get_device(&pdev->dev);
> > + dax_hmem_work.pdev = pdev;
>
> Using the pdev rather than dev breaks the pattern of doing a get_device()
> and assigning in one line. This is a bit ugly.
>
> dax_hmem_work.pdev = to_pci_dev(get_device(&pdev->dev));
>
> but perhaps makes the association tighter than current code.
>
> > + INIT_WORK(&dax_hmem_work.work, process_defer_work);
>
> See above. I think assigning the work function should be static
> which should resolve the issue Alison was seeing as then it should
> be fine to call flush_work() on the item that isn't on a work queue
> yet but is initialized.
>
> > + }
> > +
> > return walk_hmem_resources(&pdev->dev, hmem_register_device);
> > }
> >
> > @@ -162,6 +219,11 @@ static __init int dax_hmem_init(void)
> >
> > static __exit void dax_hmem_exit(void)
> > {
> > + flush_work(&dax_hmem_work.work);
>
> I think this needs to be under the if (dax_hmem_work.pdev)
> Not sure there is any guarantee dax_hmem_platform_probe() has run
> before we get here otherwise. Alternative is to assign
> the work function statically.
>
>
>
> > +
> > + if (dax_hmem_work.pdev)
> > + put_device(&dax_hmem_work.pdev->dev);
> > +
> > platform_driver_unregister(&dax_hmem_driver);
> > platform_driver_unregister(&dax_hmem_platform_driver);
> > }
>
