[PATCH v3 0/2] cxl/memdev: Fix poison debugfs vs unbind deadlock

[PATCH v3 0/2] cxl/memdev: Fix poison debugfs vs unbind deadlock

[Guixin Liu]

Writing a memdev poison debugfs file while cxl_mem is being unbound
deadlocks. Patch 2 fixes it by not waiting for the device lock in those
handlers. Patch 1 adds the trylock guard it uses.

Patch 2 does not build without patch 1, so the two need to travel
together.

Testing:

Reproduced on a QEMU CXL topology whose type3 devices advertise poison
inject support:

  while :; do echo 0 > /sys/kernel/debug/cxl/mem0/inject_poison; done &
  while :; do
      echo mem0 > /sys/bus/cxl/drivers/cxl_mem/unbind
      echo mem0 > /sys/bus/cxl/drivers/cxl_mem/bind
  done

Without the fix the unbind wedges within seconds. The three tasks
involved, from /proc/<pid>/stack:

  writer, state S, holds the debugfs reference and waits for the lock
    cxl_debugfs_poison_inject+0x25/0xa0 [cxl_mem]
    debugfs_attr_write+0x61/0xb0
    full_proxy_write+0xfc/0x1c0
    vfs_write+0x1d4/0xe60

  unbind, state D, holds the lock and waits for the reference to drain
    remove_one+0x27f/0x3d0
    debugfs_remove+0x44/0x60
    release_nodes+0xfa/0x2c0
    devres_release_all+0x113/0x1a0
    device_unbind_cleanup+0x76/0x260
    device_release_driver_internal+0x3eb/0x540
    unbind_store+0xde/0x100

  cxl_port workqueue, state D, blocked on the same lock
    device_release_driver_internal+0x96/0x540
    detach_memdev+0x79/0xb0 [cxl_core]
    process_one_work+0x6b0/0xfb0

The writer is in interruptible sleep and can be killed; the unbind
cannot, and because cxl_bus_wq is an ordered workqueue the wedged
detach_memdev() blocks every other CXL bus work item behind it.

With both patches applied, 46 unbind/bind cycles against the same writer
loop all completed, no task was left in D state, and the writer collected
10920 EBUSY returns from the contended trylock. Note that lockdep stays
quiet either way: one leg of the cycle is the debugfs active_users
completion rather than a lock it tracks.

v1 -> v2:
- add the device_trylock() guard and use ACQUIRE(device_try, ...) instead
  of open-coding device_trylock()/device_unlock(), keeping the style the
  Fixes: commit established (Shaikh Kamaluddin)
- cut the changelog down to the failing condition, the consequence and
  the fix; the call graph and the reproducer live here instead
- say how the issue was found and how it was tested

v2 -> v3:
- rebase onto v7.3-rc2 (master), per Dave's request to send the series
  against Linus's tags rather than cxl/next

v1:
https://lore.kernel.org/linux-cxl/20260826125248.4003792-1-kanie@linux.alibaba.com/

v2:
https://lore.kernel.org/linux-cxl/20260831124809.889829-1-kanie@linux.alibaba.com/

Guixin Liu (2):
  driver core: Add conditional guard support for device_trylock()
  cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

 drivers/cxl/mem.c      | 14 ++++++++++----
 include/linux/device.h |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)


base-commit: 50d05c7c76c96b90462f24debacca971d2e86713
--
2.43.7

[PATCH v3 1/2] driver core: Add conditional guard support for device_trylock()

[Guixin Liu]

Introduce a conditional guard version of device_trylock() for scenarios
that must not block on the device lock. device_trylock() returns 1 on
success like mutex_trylock(), so the default binary condition applies and
ACQUIRE_ERR() reports -EBUSY on contention.

Suggested-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 include/linux/device.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index aee79fd6b32b..a8d6cf76d137 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1121,6 +1121,7 @@ static inline void device_unlock(struct device *dev)
 
 DEFINE_GUARD(device, struct device *, device_lock(_T), device_unlock(_T))
 DEFINE_GUARD_COND(device, _intr, device_lock_interruptible(_T), _RET == 0)
+DEFINE_GUARD_COND(device, _try, device_trylock(_T))
 
 static inline void device_lock_assert(struct device *dev)
 {
-- 
2.43.7

[PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Guixin Liu]

The poison debugfs handlers take the memdev device lock so that the
region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
on the file across the handler, and cxl_mem unbind removes that file
while holding the very same device lock, so a handler that waits for the
lock deadlocks against a concurrent unbind.

Both tasks then hang. The unbind side is uninterruptible, and it also
blocks the memdev detach work, which runs on an ordered workqueue and so
stalls every other CXL bus work item.

Take the lock with the trylock guard and return -EBUSY instead of
waiting. An unbind that wins the race removes the file first and the
write fails with -ENOENT.

Found by code inspection. Reproduced by writing inject_poison in a loop
while unbinding and rebinding cxl_mem, and confirmed fixed by the same
test.

Fixes: 574eda81d0a7 ("cxl/memdev: Hold memdev lock during memdev poison injection/clear")
Suggested-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
checkpatch reports "do not use assignment in if condition" twice, on the
two ACQUIRE_ERR() lines. Those are pre-existing: the unpatched file and
the Fixes: commit report the same two, this patch only swaps the lock
class on them, and the combined form is what all 40 ACQUIRE_ERR() call
sites in drivers/cxl use.

 drivers/cxl/mem.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 798e5c369cfc..3959ec963026 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -50,8 +50,13 @@ static int cxl_debugfs_poison_inject(void *data, u64 dpa)
 	struct cxl_memdev *cxlmd = data;
 	int rc;
 
-	ACQUIRE(device_intr, devlock)(&cxlmd->dev);
-	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+	/*
+	 * Never wait for this lock: the debugfs proxy holds a file reference
+	 * across the callback and unbind removes the file under the same
+	 * device lock, so waiting here deadlocks against unbind.
+	 */
+	ACQUIRE(device_try, devlock)(&cxlmd->dev);
+	if ((rc = ACQUIRE_ERR(device_try, &devlock)))
 		return rc;
 
 	return cxl_inject_poison(cxlmd, dpa);
@@ -65,8 +70,9 @@ static int cxl_debugfs_poison_clear(void *data, u64 dpa)
 	struct cxl_memdev *cxlmd = data;
 	int rc;
 
-	ACQUIRE(device_intr, devlock)(&cxlmd->dev);
-	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+	/* Never wait, per the inject path above. */
+	ACQUIRE(device_try, devlock)(&cxlmd->dev);
+	if ((rc = ACQUIRE_ERR(device_try, &devlock)))
 		return rc;
 
 	return cxl_clear_poison(cxlmd, dpa);
-- 
2.43.7

Re: [PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Greg Kroah-Hartman]

On Thu, Sep 10, 2026 at 05:40:17PM +0800, Guixin Liu wrote:
> The poison debugfs handlers take the memdev device lock so that the
> region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
> on the file across the handler, and cxl_mem unbind removes that file
> while holding the very same device lock, so a handler that waits for the
> lock deadlocks against a concurrent unbind.
> 
> Both tasks then hang. The unbind side is uninterruptible, and it also
> blocks the memdev detach work, which runs on an ordered workqueue and so
> stalls every other CXL bus work item.
> 
> Take the lock with the trylock guard and return -EBUSY instead of
> waiting. An unbind that wins the race removes the file first and the
> write fails with -ENOENT.
> 
> Found by code inspection. Reproduced by writing inject_poison in a loop
> while unbinding and rebinding cxl_mem, and confirmed fixed by the same
> test.

Why would anyone normally "unbind" cxl_mem at all?  That will taint
kernels soon, so you don't normally want to do that, right?

And debugfs is root-only, so this is a "root did something bad, and gets
to keep the mess", right?  This should not ever be a normal operation.

thanks,

greg k-h

Re: [PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Guixin Liu]

在 2026/9/10 17:52, Greg Kroah-Hartman 写道:
> On Thu, Sep 10, 2026 at 05:40:17PM +0800, Guixin Liu wrote:
>> The poison debugfs handlers take the memdev device lock so that the
>> region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
>> on the file across the handler, and cxl_mem unbind removes that file
>> while holding the very same device lock, so a handler that waits for the
>> lock deadlocks against a concurrent unbind.
>>
>> Both tasks then hang. The unbind side is uninterruptible, and it also
>> blocks the memdev detach work, which runs on an ordered workqueue and so
>> stalls every other CXL bus work item.
>>
>> Take the lock with the trylock guard and return -EBUSY instead of
>> waiting. An unbind that wins the race removes the file first and the
>> write fails with -ENOENT.
>>
>> Found by code inspection. Reproduced by writing inject_poison in a loop
>> while unbinding and rebinding cxl_mem, and confirmed fixed by the same
>> test.
> Why would anyone normally "unbind" cxl_mem at all?  That will taint
> kernels soon, so you don't normally want to do that, right?
>
> And debugfs is root-only, so this is a "root did something bad, and gets
> to keep the mess", right?  This should not ever be a normal operation.
Agreed, nobody should unbind cxl_mem in production. The sysfs unbind is
in the reproducer only because it's the cheapest trigger.

The window itself is not sysfs-unbind specific.
The debugfs directory is torn down from a devm action, so every path 
that ends
in device_release_driver_internal() hits the same wait: rmmod cxl_mem, 
and the
memdev detach work that PCI hot-remove schedules. That detach path is 
the third
task in the reproducer stack, the one wedged on cxl_bus_wq.
A poison write racing an rmmod or a hot-remove is not root misbehaving,
and no taint flags it either.

Best Regards,
Guixin Liu
> thanks,
>
> greg k-h

Re: [PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Greg Kroah-Hartman]

On Thu, Sep 10, 2026 at 07:28:03PM +0800, Guixin Liu wrote:
> 
> 
> 在 2026/9/10 17:52, Greg Kroah-Hartman 写道:
> > On Thu, Sep 10, 2026 at 05:40:17PM +0800, Guixin Liu wrote:
> > > The poison debugfs handlers take the memdev device lock so that the
> > > region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
> > > on the file across the handler, and cxl_mem unbind removes that file
> > > while holding the very same device lock, so a handler that waits for the
> > > lock deadlocks against a concurrent unbind.
> > > 
> > > Both tasks then hang. The unbind side is uninterruptible, and it also
> > > blocks the memdev detach work, which runs on an ordered workqueue and so
> > > stalls every other CXL bus work item.
> > > 
> > > Take the lock with the trylock guard and return -EBUSY instead of
> > > waiting. An unbind that wins the race removes the file first and the
> > > write fails with -ENOENT.
> > > 
> > > Found by code inspection. Reproduced by writing inject_poison in a loop
> > > while unbinding and rebinding cxl_mem, and confirmed fixed by the same
> > > test.
> > Why would anyone normally "unbind" cxl_mem at all?  That will taint
> > kernels soon, so you don't normally want to do that, right?
> > 
> > And debugfs is root-only, so this is a "root did something bad, and gets
> > to keep the mess", right?  This should not ever be a normal operation.
> Agreed, nobody should unbind cxl_mem in production. The sysfs unbind is
> in the reproducer only because it's the cheapest trigger.
> 
> The window itself is not sysfs-unbind specific.
> The debugfs directory is torn down from a devm action, so every path that
> ends
> in device_release_driver_internal() hits the same wait: rmmod cxl_mem, and
> the
> memdev detach work that PCI hot-remove schedules. That detach path is the
> third
> task in the reproducer stack, the one wedged on cxl_bus_wq.
> A poison write racing an rmmod or a hot-remove is not root misbehaving,
> and no taint flags it either.

But how can cxl_mem ever be removed, it doesn't live on a bus that is
hot-removable, does it?

rmmod doesn't count either, that never happens except by developers.

thanks,

greg k-h

Re: [PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Jonathan Cameron]

On Thu, 10 Sep 2026 13:58:28 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Thu, Sep 10, 2026 at 07:28:03PM +0800, Guixin Liu wrote:
> > 
> > 
> > 在 2026/9/10 17:52, Greg Kroah-Hartman 写道:  
> > > On Thu, Sep 10, 2026 at 05:40:17PM +0800, Guixin Liu wrote:  
> > > > The poison debugfs handlers take the memdev device lock so that the
> > > > region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
> > > > on the file across the handler, and cxl_mem unbind removes that file
> > > > while holding the very same device lock, so a handler that waits for the
> > > > lock deadlocks against a concurrent unbind.
> > > > 
> > > > Both tasks then hang. The unbind side is uninterruptible, and it also
> > > > blocks the memdev detach work, which runs on an ordered workqueue and so
> > > > stalls every other CXL bus work item.
> > > > 
> > > > Take the lock with the trylock guard and return -EBUSY instead of
> > > > waiting. An unbind that wins the race removes the file first and the
> > > > write fails with -ENOENT.
> > > > 
> > > > Found by code inspection. Reproduced by writing inject_poison in a loop
> > > > while unbinding and rebinding cxl_mem, and confirmed fixed by the same
> > > > test.  
> > > Why would anyone normally "unbind" cxl_mem at all?  That will taint
> > > kernels soon, so you don't normally want to do that, right?
> > > 
> > > And debugfs is root-only, so this is a "root did something bad, and gets
> > > to keep the mess", right?  This should not ever be a normal operation.  
> > Agreed, nobody should unbind cxl_mem in production. The sysfs unbind is
> > in the reproducer only because it's the cheapest trigger.
> > 
> > The window itself is not sysfs-unbind specific.
> > The debugfs directory is torn down from a devm action, so every path that
> > ends
> > in device_release_driver_internal() hits the same wait: rmmod cxl_mem, and
> > the
> > memdev detach work that PCI hot-remove schedules. That detach path is the
> > third
> > task in the reproducer stack, the one wedged on cxl_bus_wq.
> > A poison write racing an rmmod or a hot-remove is not root misbehaving,
> > and no taint flags it either.  
> 
> But how can cxl_mem ever be removed, it doesn't live on a bus that is
> hot-removable, does it?

It is effectively a child of cxl_pci and that lives on the pci bus and
definitely is hotpluggable.  Those flows should work fine so
I see this as a real if somewhat obscure bug to fix.

Jonathan



> 
> rmmod doesn't count either, that never happens except by developers.
> 
> thanks,
> 
> greg k-h

Re: [PATCH v3 1/2] driver core: Add conditional guard support for device_trylock()

[Jonathan Cameron]

On Thu, 10 Sep 2026 17:40:16 +0800
Guixin Liu <kanie@linux.alibaba.com> wrote:

> Introduce a conditional guard version of device_trylock() for scenarios
> that must not block on the device lock. device_trylock() returns 1 on
> success like mutex_trylock(), so the default binary condition applies and
> ACQUIRE_ERR() reports -EBUSY on contention.
> 
> Suggested-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Re: [PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Jonathan Cameron]

On Thu, 10 Sep 2026 17:40:17 +0800
Guixin Liu <kanie@linux.alibaba.com> wrote:

> The poison debugfs handlers take the memdev device lock so that the
> region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
> on the file across the handler, and cxl_mem unbind removes that file
> while holding the very same device lock, so a handler that waits for the
> lock deadlocks against a concurrent unbind.
> 
> Both tasks then hang. The unbind side is uninterruptible, and it also
> blocks the memdev detach work, which runs on an ordered workqueue and so
> stalls every other CXL bus work item.
> 
> Take the lock with the trylock guard and return -EBUSY instead of
> waiting. An unbind that wins the race removes the file first and the
> write fails with -ENOENT.
> 
> Found by code inspection. Reproduced by writing inject_poison in a loop
> while unbinding and rebinding cxl_mem, and confirmed fixed by the same
> test.
> 
> Fixes: 574eda81d0a7 ("cxl/memdev: Hold memdev lock during memdev poison injection/clear")
> Suggested-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Probably not one to rush in but nice to clean up the deadlock even if it
is a little hard to hit.  If it is possible to test with a hot remove
flow even better.  You should be able to do that with emulation in qemu
if you don't have hardware capable of safe hotplug operations.

> ---
> checkpatch reports "do not use assignment in if condition" twice, on the
> two ACQUIRE_ERR() lines. Those are pre-existing: the unpatched file and
> the Fixes: commit report the same two, this patch only swaps the lock
> class on them, and the combined form is what all 40 ACQUIRE_ERR() call
> sites in drivers/cxl use.

We should fix that up. Oddly I thought we had, but guess not.

> 
>  drivers/cxl/mem.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 798e5c369cfc..3959ec963026 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -50,8 +50,13 @@ static int cxl_debugfs_poison_inject(void *data, u64 dpa)
>  	struct cxl_memdev *cxlmd = data;
>  	int rc;
>  
> -	ACQUIRE(device_intr, devlock)(&cxlmd->dev);
> -	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
> +	/*
> +	 * Never wait for this lock: the debugfs proxy holds a file reference
> +	 * across the callback and unbind removes the file under the same
> +	 * device lock, so waiting here deadlocks against unbind.
> +	 */
> +	ACQUIRE(device_try, devlock)(&cxlmd->dev);
> +	if ((rc = ACQUIRE_ERR(device_try, &devlock)))
>  		return rc;
>  
>  	return cxl_inject_poison(cxlmd, dpa);
> @@ -65,8 +70,9 @@ static int cxl_debugfs_poison_clear(void *data, u64 dpa)
>  	struct cxl_memdev *cxlmd = data;
>  	int rc;
>  
> -	ACQUIRE(device_intr, devlock)(&cxlmd->dev);
> -	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
> +	/* Never wait, per the inject path above. */
> +	ACQUIRE(device_try, devlock)(&cxlmd->dev);
> +	if ((rc = ACQUIRE_ERR(device_try, &devlock)))
>  		return rc;
>  
>  	return cxl_clear_poison(cxlmd, dpa);

Re: [PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Guixin Liu]

在 2026/9/11 05:03, Jonathan Cameron 写道:
> On Thu, 10 Sep 2026 17:40:17 +0800
> Guixin Liu <kanie@linux.alibaba.com> wrote:
>
>> The poison debugfs handlers take the memdev device lock so that the
>> region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
>> on the file across the handler, and cxl_mem unbind removes that file
>> while holding the very same device lock, so a handler that waits for the
>> lock deadlocks against a concurrent unbind.
>>
>> Both tasks then hang. The unbind side is uninterruptible, and it also
>> blocks the memdev detach work, which runs on an ordered workqueue and so
>> stalls every other CXL bus work item.
>>
>> Take the lock with the trylock guard and return -EBUSY instead of
>> waiting. An unbind that wins the race removes the file first and the
>> write fails with -ENOENT.
>>
>> Found by code inspection. Reproduced by writing inject_poison in a loop
>> while unbinding and rebinding cxl_mem, and confirmed fixed by the same
>> test.
>>
>> Fixes: 574eda81d0a7 ("cxl/memdev: Hold memdev lock during memdev poison injection/clear")
>> Suggested-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
>
> Probably not one to rush in but nice to clean up the deadlock even if it
> is a little hard to hit.  If it is possible to test with a hot remove
> flow even better.  You should be able to do that with emulation in qemu
> if you don't have hardware capable of safe hotplug operations.
Sure, I reproduced this on hot-remove situation:

debugfs writer, state S, waits for the memdev lock
       cxl_debugfs_poison_inject+0x25/0xa0 [cxl_mem]
       simple_attr_write_xsigned.isra.0+0x1ca/0x2c0
       debugfs_attr_write+0x61/0xb0
       full_proxy_write+0xfc/0x1c0
       vfs_write+0x1d4/0xe60
       ksys_write+0x11f/0x250
       do_syscall_64+0xe2/0x560
       entry_SYSCALL_64_after_hwframe+0x76/0x7e

irq/28-pciehp, state D, holds the memdev lock, waits for the debugfs 
reference to drain
       remove_one+0x27f/0x3d0
       __simple_recursive_removal+0x183/0x4a0
       debugfs_remove+0x44/0x60
       release_nodes+0xfa/0x2c0
       devres_release_all+0x113/0x1a0
       device_unbind_cleanup+0x76/0x260
       device_release_driver_internal+0x3eb/0x540
       bus_remove_device+0x28a/0x540
       device_del+0x371/0x930
       cdev_device_del+0x1d/0xf0
       cxl_memdev_unregister+0x1c/0x70 [cxl_core]
       release_nodes+0xfa/0x2c0
       devres_release_all+0x113/0x1a0
       device_unbind_cleanup+0x76/0x260
       device_release_driver_internal+0x3eb/0x540
       pci_stop_bus_device+0x122/0x170
       pci_stop_and_remove_bus_device+0x16/0x30
       pciehp_unconfigure_device+0x1b4/0x3b0
       pciehp_disable_slot+0xf9/0x2e0
       pciehp_handle_disable_request+0x81/0x100
       pciehp_ist+0x29f/0x410
       irq_thread_fn+0x8b/0x160
       irq_thread+0x189/0x320
       kthread+0x329/0x410
       ret_from_fork+0x33b/0x670
       ret_from_fork_asm+0x1a/0x30

cxl_port workqueue, state D, waits for the memdev lock
       device_release_driver_internal+0x96/0x540
       detach_memdev+0x79/0xb0 [cxl_core]
       process_one_work+0x6b0/0xfb0
       worker_thread+0x4dd/0xd30
       kthread+0x329/0x410
       ret_from_fork+0x33b/0x670
       ret_from_fork_asm+0x1a/0x30

With this patch, the hot-remove flow completed normally.

>
>> ---
>> checkpatch reports "do not use assignment in if condition" twice, on the
>> two ACQUIRE_ERR() lines. Those are pre-existing: the unpatched file and
>> the Fixes: commit report the same two, this patch only swaps the lock
>> class on them, and the combined form is what all 40 ACQUIRE_ERR() call
>> sites in drivers/cxl use.
> We should fix that up. Oddly I thought we had, but guess not.
I think we should fix this in checkpatch.pl, like this:
     if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s &&
    +    $c !~ /=\s*ACQUIRE_ERR\s*\(/) {

Best Regards,
Guixin Liu
>>   drivers/cxl/mem.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>> index 798e5c369cfc..3959ec963026 100644
>> --- a/drivers/cxl/mem.c
>> +++ b/drivers/cxl/mem.c
>> @@ -50,8 +50,13 @@ static int cxl_debugfs_poison_inject(void *data, u64 dpa)
>>   	struct cxl_memdev *cxlmd = data;
>>   	int rc;
>>   
>> -	ACQUIRE(device_intr, devlock)(&cxlmd->dev);
>> -	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
>> +	/*
>> +	 * Never wait for this lock: the debugfs proxy holds a file reference
>> +	 * across the callback and unbind removes the file under the same
>> +	 * device lock, so waiting here deadlocks against unbind.
>> +	 */
>> +	ACQUIRE(device_try, devlock)(&cxlmd->dev);
>> +	if ((rc = ACQUIRE_ERR(device_try, &devlock)))
>>   		return rc;
>>   
>>   	return cxl_inject_poison(cxlmd, dpa);
>> @@ -65,8 +70,9 @@ static int cxl_debugfs_poison_clear(void *data, u64 dpa)
>>   	struct cxl_memdev *cxlmd = data;
>>   	int rc;
>>   
>> -	ACQUIRE(device_intr, devlock)(&cxlmd->dev);
>> -	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
>> +	/* Never wait, per the inject path above. */
>> +	ACQUIRE(device_try, devlock)(&cxlmd->dev);
>> +	if ((rc = ACQUIRE_ERR(device_try, &devlock)))
>>   		return rc;
>>   
>>   	return cxl_clear_poison(cxlmd, dpa);

Re: [PATCH v3 2/2] cxl/memdev: Fix deadlock between poison debugfs and cxl_mem unbind

[Jonathan Cameron]

On Fri, 11 Sep 2026 11:04:47 +0800
Guixin Liu <kanie@linux.alibaba.com> wrote:

> 在 2026/9/11 05:03, Jonathan Cameron 写道:
> > On Thu, 10 Sep 2026 17:40:17 +0800
> > Guixin Liu <kanie@linux.alibaba.com> wrote:
> >  
> >> The poison debugfs handlers take the memdev device lock so that the
> >> region lookup sees a stable cxlmd->dev.driver. debugfs holds a reference
> >> on the file across the handler, and cxl_mem unbind removes that file
> >> while holding the very same device lock, so a handler that waits for the
> >> lock deadlocks against a concurrent unbind.
> >>
> >> Both tasks then hang. The unbind side is uninterruptible, and it also
> >> blocks the memdev detach work, which runs on an ordered workqueue and so
> >> stalls every other CXL bus work item.
> >>
> >> Take the lock with the trylock guard and return -EBUSY instead of
> >> waiting. An unbind that wins the race removes the file first and the
> >> write fails with -ENOENT.
> >>
> >> Found by code inspection. Reproduced by writing inject_poison in a loop
> >> while unbinding and rebinding cxl_mem, and confirmed fixed by the same
> >> test.
> >>
> >> Fixes: 574eda81d0a7 ("cxl/memdev: Hold memdev lock during memdev poison injection/clear")
> >> Suggested-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> >> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>  
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> >
> > Probably not one to rush in but nice to clean up the deadlock even if it
> > is a little hard to hit.  If it is possible to test with a hot remove
> > flow even better.  You should be able to do that with emulation in qemu
> > if you don't have hardware capable of safe hotplug operations.  
> Sure, I reproduced this on hot-remove situation:
> 
> debugfs writer, state S, waits for the memdev lock
>        cxl_debugfs_poison_inject+0x25/0xa0 [cxl_mem]
...

> With this patch, the hot-remove flow completed normally.

Nice. Thanks for doing that.

> 
> >  
> >> ---
> >> checkpatch reports "do not use assignment in if condition" twice, on the
> >> two ACQUIRE_ERR() lines. Those are pre-existing: the unpatched file and
> >> the Fixes: commit report the same two, this patch only swaps the lock
> >> class on them, and the combined form is what all 40 ACQUIRE_ERR() call
> >> sites in drivers/cxl use.  
> > We should fix that up. Oddly I thought we had, but guess not.  
> I think we should fix this in checkpatch.pl, like this:
>      if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s &&
>     +    $c !~ /=\s*ACQUIRE_ERR\s*\(/) {

There are a few other macros that are wrappers of ACQUIRE_ERR
that should be covered in such a patch as well.  If you have
time send a patch!

Thanks,

Jonathan