mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Koralahalli Channabasappa, Smita" <skoralah@amd.com>
To: Alison Schofield <alison.schofield@intel.com>,
	Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>,
	Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: linux-cxl@vger.kernel.org, linux-kernel@vger.kernel.org,
	nvdimm@lists.linux.dev, linux-fsdevel@vger.kernel.org,
	linux-pm@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Yazen Ghannam <yazen.ghannam@amd.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Matthew Wilcox <willy@infradead.org>, Jan Kara <jack@suse.cz>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@kernel.org>,
	Li Ming <ming.li@zohomail.com>,
	Jeff Johnson <jeff.johnson@oss.qualcomm.com>,
	Ying Huang <huang.ying.caritas@gmail.com>,
	Yao Xingtao <yaoxt.fnst@fujitsu.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Nathan Fontenot <nathan.fontenot@amd.com>,
	Terry Bowman <terry.bowman@amd.com>,
	Robert Richter <rrichter@amd.com>,
	Benjamin Cheatham <benjamin.cheatham@amd.com>,
	Zhijian Li <lizhijian@fujitsu.com>,
	Borislav Petkov <bp@alien8.de>,
	Tomasz Wolski <tomasz.wolski@fujitsu.com>
Subject: Re: [PATCH v7 3/7] dax/cxl, hmem: Initialize hmem early and defer dax_cxl binding
Date: Thu, 19 Mar 2026 09:45:38 -0700	[thread overview]
Message-ID: <b56f55b1-4281-4edf-8aa4-27d0500ebd60@amd.com> (raw)
In-Reply-To: <3590e2d5-e768-4180-82a0-c972101f3440@amd.com>

On 3/19/2026 8:46 AM, Koralahalli Channabasappa, Smita wrote:
> Hi Jonathan and Alison,
> 
> Thanks for the report and suggestions. I took a look at Jonathan's 
> comments in Patch 6 and tying it together here.
> 
> On 3/18/2026 10:48 PM, Alison Schofield wrote:
>> On Thu, Mar 19, 2026 at 01:14:56AM +0000, Smita Koralahalli wrote:
>>> From: Dan Williams <dan.j.williams@intel.com>
>>>
>>> Move hmem/ earlier in the dax Makefile so that hmem_init() runs before
>>> dax_cxl.
>>>
>>> In addition, defer registration of the dax_cxl driver to a workqueue
>>> instead of using module_cxl_driver(). This ensures that dax_hmem has
>>> an opportunity to initialize and register its deferred callback and make
>>> ownership decisions before dax_cxl begins probing and claiming Soft
>>> Reserved ranges.
>>>
>>> Mark the dax_cxl driver as PROBE_PREFER_ASYNCHRONOUS so its probe runs
>>> out of line from other synchronous probing avoiding ordering
>>> dependencies while coordinating ownership decisions with dax_hmem.
>>
>> Hi Smita,
>>
>> Replying to this patch, as it's my best guess as to why I may be
>> seeing this WARN when I modprobe cxl-test.
>>
>> We are able to pass all the CXL unit tests because it is only that
>> first load that causes the WARN. All subsequent reloads of cxl-test
>> do not unload dax_cxl and dax_hmem so they chug happily along.
>>
>> I can reproduce by unloading each piece before reloading cxl-test
>> # modprobe -r cxl-test
>> # modprobe -r dax_cxl
>> # modprobe -r dax_hmem
>> # modprobe cxl-test
>> and the WARN repeats.
>>
>> Guessing you may recognize what is going on. Let me know if I can
>> try anything else out.
>>
>>
>> # dmesg (trimmed to just the init calls)
>> [   34.229033] calling  fwctl_init+0x0/0xff0 [fwctl] @ 1057
>> [   34.230616] initcall fwctl_init+0x0/0xff0 [fwctl] returned 0 after 
>> 186 usecs
>> [   34.257096] calling  cxl_core_init+0x0/0x100 [cxl_core] @ 1057
>> [   34.258395] initcall cxl_core_init+0x0/0x100 [cxl_core] returned 0 
>> after 538 usecs
>> [   34.264170] calling  cxl_port_init+0x0/0xff0 [cxl_port] @ 1057
>> [   34.264982] initcall cxl_port_init+0x0/0xff0 [cxl_port] returned 0 
>> after 110 usecs
>> [   34.268058] calling  cxl_mem_driver_init+0x0/0xff0 [cxl_mem] @ 1057
>> [   34.268743] initcall cxl_mem_driver_init+0x0/0xff0 [cxl_mem] 
>> returned 0 after 110 usecs
>> [   34.274670] calling  cxl_pmem_init+0x0/0xff0 [cxl_pmem] @ 1057
>> [   34.277835] initcall cxl_pmem_init+0x0/0xff0 [cxl_pmem] returned 0 
>> after 1671 usecs
>> [   34.285807] calling  cxl_acpi_init+0x0/0xff0 [cxl_acpi] @ 1057
>> [   34.287105] initcall cxl_acpi_init+0x0/0xff0 [cxl_acpi] returned 0 
>> after 262 usecs
>> [   34.292967] calling  cxl_test_init+0x0/0xff0 [cxl_test] @ 1057
>> [   34.339841] initcall cxl_test_init+0x0/0xff0 [cxl_test] returned 0 
>> after 45832 usecs
>> [   34.342259] calling  cxl_mock_mem_driver_init+0x0/0xff0 
>> [cxl_mock_mem] @ 1063
>> [   34.343459] initcall cxl_mock_mem_driver_init+0x0/0xff0 
>> [cxl_mock_mem] returned 0 after 356 usecs
>> [   34.658602] calling  dax_hmem_init+0x0/0xff0 [dax_hmem] @ 1059
>> [   34.670106] calling  cxl_pci_driver_init+0x0/0xff0 [cxl_pci] @ 1100
>> [   34.671023] initcall cxl_pci_driver_init+0x0/0xff0 [cxl_pci] 
>> returned 0 after 197 usecs
>> [   34.673051] initcall dax_hmem_init+0x0/0xff0 [dax_hmem] returned 0 
>> after 2225 usecs
> 
> I agree with Jonathan's comments in Patch 6, using __WORK_INITIALIZER or 
> initializing work in dax_hmem_init() and gating flush on pdev will fix 
> the WARN — I will add both for v8. But I think the WARN is likely 
> indicating an ordering issue here..
> 
> On initial boot, the Makefile ordering ensures dax_hmem_init() runs
> before cxl_dax_region_init(), so both work items land on system_long_wq
> in the right order and dax_hmem's deferred work is queued before 
> dax_cxl's driver registration work.
> 
> On module reload which Alison is trying here I dont think, modules are 
> loaded by Makefile order. I think dax_cxl's workqueue is calling 
> dax_hmem_flush_work() before dax_hmem probe has had a chance to queue 
> its work, so flush_work() flushes nothing and dax_cxl registers its 
> driver without waiting.
> 
> __WORK_INITIALIZER fixes the WARN, but doesn't fix the race I guess if 
> we are hitting that here..
> 
> [   34.673051] initcall dax_hmem_init+0x0/0xff0 [dax_hmem] returned 0 
> after 2225 usecs
> [   34.676011] calling  cxl_dax_region_init+0x0/0xff0 [dax_cxl] @ 1059
> 
> These two lines indicate cxl_dax started after dax_hmem_init() returns 
> but I dont think that guarantees dax_hmem_platform_probe() has actually 
> run..
> 
> I dont know if wait_for_device_probe() in cxl_dax_region_driver_register
> might help..
> 
> Thanks
> Smita

Actually, thinking about this more..

dax_hmem_initial_probe lives in device.c (built-in) so it survives 
module reload. On reload it's still true from the first boot. This means 
hmem_register_device() skips the deferral path entirely..

The problem is this bypasses the cxl_region_contains_resource() check 
that the deferred work normally does. On first boot, 
process_defer_work() walks each range and decides per-range: if CXL 
covers it, skip. If not, register with HMEM. On reload, that check never 
happens — whoever registers first via alloc_dax_region() wins, 
regardless of whether CXL actually covers the range.

So if dax_cxl registers first on reload, it could claim a range that CXL 
doesn't actually cover, and dax_hmem would lose a range it should own..

I dont know if Im thinking through this right..

Thanks
Smita

> 
>> [   34.676011] calling  cxl_dax_region_init+0x0/0xff0 [dax_cxl] @ 1059
>> [   34.676856] ------------[ cut here ]------------
>> [   34.677533] WARNING: kernel/workqueue.c:4289 at 
>> __flush_work+0x4f9/0x550, CPU#3: kworker/3:2/136
>> [   34.678596] Modules linked in: dax_cxl(+) cxl_pci dax_hmem 
>> cxl_mock_mem(O) cxl_test(O) cxl_acpi(O) cxl_pmem(O) cxl_mem(O) 
>> cxl_port(O) cxl_mock(O) cxl_core(O) fwctl nd_pmem nd_btt dax_pmem nfit 
>> nd_e820 libnvdimm
>> [   34.680632] initcall cxl_dax_region_init+0x0/0xff0 [dax_cxl] 
>> returned 0 after 3842 usecs
>> [   34.680918] CPU: 3 UID: 0 PID: 136 Comm: kworker/3:2 Tainted: 
>> G           O        7.0.0-rc4+ #156 PREEMPT(full)
>> [   34.684368] Tainted: [O]=OOT_MODULE
>> [   34.684993] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), 
>> BIOS 0.0.0 02/06/2015
>> [   34.686098] Workqueue: events_long cxl_dax_region_driver_register 
>> [dax_cxl]
>> [   34.687108] RIP: 0010:__flush_work+0x4f9/0x550
>>
>> That addr is this line in flush_work()
>>          if (WARN_ON(!work->func))
>>                  return false;
>>
>>
>> [   34.687811] Code: ff 49 8b 45 00 49 8b 55 08 89 c7 48 c1 e8 04 83 
>> e7 08 83 e0 0f 83 cf 02 49 0f ba 6d 00 03 e9 a1 fc ff ff 0f 0b e9 e6 
>> fe ff ff <0f> 0b e9 df fe ff ff e8 9b 48 15 01 85 c0 0f 84 26 ff ff ff 
>> 80 3d
>> [   34.690107] RSP: 0018:ffffc900020b7cf8 EFLAGS: 00010246
>> [   34.690673] RAX: 0000000000000000 RBX: ffffffffa0ea2088 RCX: 
>> ffff8880088b2b78
>> [   34.691388] RDX: 00000000834fb194 RSI: 0000000000000000 RDI: 
>> ffffffffa0ea2088
>> [   34.692135] RBP: ffffc900020b7de0 R08: 0000000031ab93b0 R09: 
>> 00000000effb42e8
>> [   34.692876] R10: 000000008effb42e R11: 0000000000000000 R12: 
>> ffff88807d9bb340
>> [   34.693588] R13: ffffffffa0ea2088 R14: ffffffffa0ed2020 R15: 
>> 0000000000000001
>> [   34.694358] FS:  0000000000000000(0000) GS:ffff8880fa45f000(0000) 
>> knlGS:0000000000000000
>> [   34.695179] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [   34.695775] CR2: 00007fe888b4e34c CR3: 00000000090ed004 CR4: 
>> 0000000000370ef0
>> [   34.696494] Call Trace:
>> [   34.696889]  <TASK>
>> [   34.697238]  ? __lock_acquire+0xb08/0x2930
>> [   34.697730]  ? __this_cpu_preempt_check+0x13/0x20
>> [   34.698277]  flush_work+0x17/0x30
>> [   34.698705]  dax_hmem_flush_work+0x10/0x20 [dax_hmem]
>> [   34.699270]  cxl_dax_region_driver_register+0x9/0x30 [dax_cxl]
>> [   34.699943]  process_one_work+0x203/0x6c0
>> [   34.700452]  worker_thread+0x197/0x350
>> [   34.700942]  ? __pfx_worker_thread+0x10/0x10
>> [   34.701455]  kthread+0x108/0x140
>> [   34.701915]  ? __pfx_kthread+0x10/0x10
>> [   34.702396]  ret_from_fork+0x28a/0x310
>> [   34.702880]  ? __pfx_kthread+0x10/0x10
>> [   34.703363]  ret_from_fork_asm+0x1a/0x30
>> [   34.703872]  </TASK>
>> [   34.704227] irq event stamp: 11015
>> [   34.704656] hardirqs last  enabled at (11025): [<ffffffff813486de>] 
>> __up_console_sem+0x5e/0x80
>> [   34.705493] hardirqs last disabled at (11036): [<ffffffff813486c3>] 
>> __up_console_sem+0x43/0x80
>> [   34.706354] softirqs last  enabled at (10500): [<ffffffff812ab9f3>] 
>> __irq_exit_rcu+0xc3/0x120
>> [   34.707197] softirqs last disabled at (10495): [<ffffffff812ab9f3>] 
>> __irq_exit_rcu+0xc3/0x120
>> [   34.708015] ---[ end trace 0000000000000000 ]---
>> [   34.752127] calling  dax_init+0x0/0xff0 [device_dax] @ 1089
>> [   34.754006] initcall dax_init+0x0/0xff0 [device_dax] returned 0 
>> after 422 usecs
>> [   34.759609] calling  dax_kmem_init+0x0/0xff0 [kmem] @ 1089
>> [   37.338377] initcall dax_kmem_init+0x0/0xff0 [kmem] returned 0 
>> after 2577658 usecs
>>
>>
>>>
>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>> Signed-off-by: Smita Koralahalli 
>>> <Smita.KoralahalliChannabasappa@amd.com>
>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>>> ---
>>>   drivers/dax/Makefile |  3 +--
>>>   drivers/dax/cxl.c    | 27 ++++++++++++++++++++++++++-
>>>   2 files changed, 27 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/dax/Makefile b/drivers/dax/Makefile
>>> index 5ed5c39857c8..70e996bf1526 100644
>>> --- a/drivers/dax/Makefile
>>> +++ b/drivers/dax/Makefile
>>> @@ -1,4 +1,5 @@
>>>   # SPDX-License-Identifier: GPL-2.0
>>> +obj-y += hmem/
>>>   obj-$(CONFIG_DAX) += dax.o
>>>   obj-$(CONFIG_DEV_DAX) += device_dax.o
>>>   obj-$(CONFIG_DEV_DAX_KMEM) += kmem.o
>>> @@ -10,5 +11,3 @@ dax-y += bus.o
>>>   device_dax-y := device.o
>>>   dax_pmem-y := pmem.o
>>>   dax_cxl-y := cxl.o
>>> -
>>> -obj-y += hmem/
>>> diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
>>> index 13cd94d32ff7..a2136adfa186 100644
>>> --- a/drivers/dax/cxl.c
>>> +++ b/drivers/dax/cxl.c
>>> @@ -38,10 +38,35 @@ static struct cxl_driver cxl_dax_region_driver = {
>>>       .id = CXL_DEVICE_DAX_REGION,
>>>       .drv = {
>>>           .suppress_bind_attrs = true,
>>> +        .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>>       },
>>>   };
>>> -module_cxl_driver(cxl_dax_region_driver);
>>> +static void cxl_dax_region_driver_register(struct work_struct *work)
>>> +{
>>> +    cxl_driver_register(&cxl_dax_region_driver);
>>> +}
>>> +
>>> +static DECLARE_WORK(cxl_dax_region_driver_work, 
>>> cxl_dax_region_driver_register);
>>> +
>>> +static int __init cxl_dax_region_init(void)
>>> +{
>>> +    /*
>>> +     * Need to resolve a race with dax_hmem wanting to drive regions
>>> +     * instead of CXL
>>> +     */
>>> +    queue_work(system_long_wq, &cxl_dax_region_driver_work);
>>> +    return 0;
>>> +}
>>> +module_init(cxl_dax_region_init);
>>> +
>>> +static void __exit cxl_dax_region_exit(void)
>>> +{
>>> +    flush_work(&cxl_dax_region_driver_work);
>>> +    cxl_driver_unregister(&cxl_dax_region_driver);
>>> +}
>>> +module_exit(cxl_dax_region_exit);
>>> +
>>>   MODULE_ALIAS_CXL(CXL_DEVICE_DAX_REGION);
>>>   MODULE_DESCRIPTION("CXL DAX: direct access to CXL regions");
>>>   MODULE_LICENSE("GPL");
>>> -- 
>>> 2.17.1
>>>
>>>
> 


  reply	other threads:[~2026-03-19 16:45 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-19  1:14 [PATCH v7 0/7] dax/hmem, cxl: Coordinate Soft Reserved handling with CXL and HMEM Smita Koralahalli
2026-03-19  1:14 ` [PATCH v7 1/7] dax/hmem: Request cxl_acpi and cxl_pci before walking Soft Reserved ranges Smita Koralahalli
2026-03-19  1:14 ` [PATCH v7 2/7] dax/hmem: Gate Soft Reserved deferral on DEV_DAX_CXL Smita Koralahalli
2026-03-19  1:14 ` [PATCH v7 3/7] dax/cxl, hmem: Initialize hmem early and defer dax_cxl binding Smita Koralahalli
2026-03-19  5:48   ` Alison Schofield
2026-03-19 14:11     ` Jonathan Cameron
2026-03-19 15:46     ` Koralahalli Channabasappa, Smita
2026-03-19 16:45       ` Koralahalli Channabasappa, Smita [this message]
2026-03-19 23:07         ` Dan Williams
2026-03-20 17:29           ` Koralahalli Channabasappa, Smita
2026-03-20 20:42           ` Koralahalli Channabasappa, Smita
2026-03-19  1:14 ` [PATCH v7 4/7] dax: Track all dax_region allocations under a global resource tree Smita Koralahalli
2026-03-19 13:59   ` Jonathan Cameron
2026-03-20 16:58     ` Koralahalli Channabasappa, Smita
2026-03-19  1:14 ` [PATCH v7 5/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions Smita Koralahalli
2026-03-19  1:14 ` [PATCH v7 6/7] dax/hmem, cxl: Defer and resolve Soft Reserved ownership Smita Koralahalli
2026-03-19 14:29   ` Jonathan Cameron
2026-03-19 20:03     ` Alison Schofield
2026-03-20 17:17     ` Koralahalli Channabasappa, Smita
2026-03-19  1:15 ` [PATCH v7 7/7] dax/hmem: Reintroduce Soft Reserved ranges back into the iomem tree Smita Koralahalli
2026-03-19 14:35   ` Jonathan Cameron
2026-03-20 17:00     ` Koralahalli Channabasappa, Smita

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b56f55b1-4281-4edf-8aa4-27d0500ebd60@amd.com \
    --to=skoralah@amd.com \
    --cc=Smita.KoralahalliChannabasappa@amd.com \
    --cc=alison.schofield@intel.com \
    --cc=ardb@kernel.org \
    --cc=benjamin.cheatham@amd.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=huang.ying.caritas@gmail.com \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=jeff.johnson@oss.qualcomm.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=len.brown@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lizhijian@fujitsu.com \
    --cc=ming.li@zohomail.com \
    --cc=nathan.fontenot@amd.com \
    --cc=nvdimm@lists.linux.dev \
    --cc=pavel@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=rrichter@amd.com \
    --cc=terry.bowman@amd.com \
    --cc=tomasz.wolski@fujitsu.com \
    --cc=vishal.l.verma@intel.com \
    --cc=willy@infradead.org \
    --cc=yaoxt.fnst@fujitsu.com \
    --cc=yazen.ghannam@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome