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

[Jonathan Cameron <jic23@kernel.org>]

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);