aie2_cmd_wait() waits uninterruptibly and without a timeout. Commit
c8d2530791cb ("accel/amdxdna: Fix deadlock on debug BO command timeout")
drops dev_lock across it, but the callers still hold client_lock, which is
device wide: while a debug BO command is outstanding nobody can open the
device, close their DRM file, or unload the module. tdr_timeout_ms=0 makes
that unbounded, and the task cannot be killed to release the lock.
Wait interruptibly. Return -EINTR, not -ERESTARTSYS: the command stays in
flight and a restarted ioctl would submit a second one. An interrupted
ATTACH leaves the outcome unknown, so record the BO as attached, since
DETACH is refused otherwise.
Fixes: 7ea046838021 ("accel/amdxdna: Support firmware debug buffer")
Signed-off-by: Taimuraz Kaitmazov <[email protected]>
---
v2:
- Rebased onto drm-misc-fixes, as you asked; the deadlock reasoning in v1
is gone with it. Please also disregard my follow-up question on the v1
thread, it asked about a shape you had already taken.
- Added the Fixes: tag c8d2530791cb carries for this function.
- Return -EINTR instead of -ERESTARTSYS.
- Record the debug BO as attached when the wait is interrupted.
- Leave the !out_fence path returning success, so only the wait changes.
drivers/accel/amdxdna/aie2_ctx.c | 33 +++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
index b713a57b3a3b..175fceea5c8f 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -908,20 +908,30 @@ static int aie2_hwctx_cu_config(struct amdxdna_hwctx
*hwctx, void *buf, u32 size
return ret;
}
-static void aie2_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq)
+static int aie2_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq)
{
struct dma_fence *out_fence = aie2_cmd_get_out_fence(hwctx, seq);
struct amdxdna_dev *xdna = hwctx->client->xdna;
+ long ret;
if (!out_fence) {
XDNA_ERR(xdna, "Failed to get fence");
- return;
+ return 0;
}
mutex_unlock(&xdna->dev_lock);
- dma_fence_wait_timeout(out_fence, false, MAX_SCHEDULE_TIMEOUT);
+ ret = dma_fence_wait_timeout(out_fence, true, MAX_SCHEDULE_TIMEOUT);
mutex_lock(&xdna->dev_lock);
dma_fence_put(out_fence);
+
+ /*
+ * The command is still in flight. If the ioctl were restarted it
+ * would submit a second one.
+ */
+ if (ret == -ERESTARTSYS)
+ return -EINTR;
+
+ return ret < 0 ? ret : 0;
}
static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl,
@@ -968,7 +978,17 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx
*hwctx, u32 bo_hdl,
goto put_cmd;
}
- aie2_cmd_wait(hwctx, seq);
+ ret = aie2_cmd_wait(hwctx, seq);
+ if (ret) {
+ /*
+ * The command may still attach the BO. Record it as attached,
+ * or DETACH will refuse it and there is no way back.
+ */
+ if (attach)
+ abo->assigned_hwctx = hwctx->id;
+ goto put_cmd;
+ }
+
if (cmd->result) {
XDNA_ERR(xdna, "Response failure 0x%x", cmd->result);
ret = -EINVAL;
@@ -1028,7 +1048,10 @@ int aie2_hwctx_sync_debug_bo(struct amdxdna_hwctx
*hwctx, u32 debug_bo_hdl)
goto put_cmd;
}
- aie2_cmd_wait(hwctx, seq);
+ ret = aie2_cmd_wait(hwctx, seq);
+ if (ret)
+ goto put_cmd;
+
if (cmd->result) {
XDNA_ERR(xdna, "Response failure 0x%x", cmd->result);
ret = -EINVAL;
--
2.55.0
