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

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

[Guixin Liu]

Writing to the memdev poison debugfs files while cxl_mem is being unbound
deadlocks. The unbind path takes the memdev device lock and then waits for
the debugfs file to drain, while the writer holds the debugfs reference and
waits for that same lock:

  unbind_store()                     echo 0 > .../mem0/inject_poison
    device_release_driver_internal()   full_proxy_write()
      <takes cxlmd->dev lock>            <takes debugfs_file_get() ref>
      device_unbind_cleanup()            debugfs_attr_write()
        devres_release_all()               cxl_debugfs_poison_inject()
          remove_debugfs()                   <waits for cxlmd->dev lock>
            debugfs_remove_recursive()
              __debugfs_file_removed()
                <waits for the ref to drain>

Neither side can make progress. A third task piles up behind the held
device lock: detach_memdev() runs on cxl_bus_wq, and since that is an
ordered workqueue the wedged work item stalls every other CXL bus work
item, so unrelated devices stop being rescanned or detached.

The write side only needs the device lock so that cxl_dpa_to_region() sees
a stable cxlmd->dev.driver. Nothing requires the caller to wait for the
lock, so use device_trylock() and report -EBUSY instead. That keeps the
debugfs reference from ever being held across a wait for the lock, which
breaks the cycle. A concurrent unbind now either completes before the write
starts, in which case the write fails with -ENOENT because the file is
gone, or loses the race and the write reports -EBUSY.

The unbind task sits in uninterruptible sleep and cannot be killed. The
writer can be signalled, which is the only reason the current code is
recoverable at all.

Reproducer, on a device that advertises 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

This hits within milliseconds. Note that lockdep stays quiet: one end of
the cycle is the debugfs active_users completion rather than a lock it
tracks. The hung task detector does report both blocked tasks.

Fixes: 574eda81d0a7 ("cxl/memdev: Hold memdev lock during memdev poison injection/clear")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/mem.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 798e5c369cfc..01d2d5855694 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -50,11 +50,20 @@ 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)))
-		return rc;
+	/*
+	 * The debugfs proxy holds a debugfs_file_get() reference across this
+	 * callback, and cxl_mem unbind removes this file from devres while
+	 * holding the same device lock. Blocking here would let unbind wait
+	 * in debugfs_remove() for a reference that cannot be dropped until
+	 * unbind releases the lock, so never wait for the lock.
+	 */
+	if (!device_trylock(&cxlmd->dev))
+		return -EBUSY;
 
-	return cxl_inject_poison(cxlmd, dpa);
+	rc = cxl_inject_poison(cxlmd, dpa);
+	device_unlock(&cxlmd->dev);
+
+	return rc;
 }
 
 DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
@@ -65,11 +74,14 @@ 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)))
-		return rc;
+	/* Avoid the unbind deadlock described in the inject path above. */
+	if (!device_trylock(&cxlmd->dev))
+		return -EBUSY;
+
+	rc = cxl_clear_poison(cxlmd, dpa);
+	device_unlock(&cxlmd->dev);
 
-	return cxl_clear_poison(cxlmd, dpa);
+	return rc;
 }
 
 DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
-- 
2.43.7