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

[Jonathan Cameron <jic23@kernel.org>]

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