| Message ID | 20230517133408.210944-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ublk: fix AB-BA lockdep warning | expand |
On May 17, 2023 / 21:34, Ming Lei wrote: > When handling UBLK_IO_FETCH_REQ, ctx->uring_lock is grabbed first, then > ub->mutex is acquired. > > When handling UBLK_CMD_STOP_DEV or UBLK_CMD_DEL_DEV, ub->mutex is > grabbed first, then calling io_uring_cmd_done() for canceling uring > command, in which ctx->uring_lock may be required. > > Real deadlock only happens when all the above commands are issued from > same uring context, and in reality different uring contexts are often used > for handing control command and IO command. > > Fix the issue by using io_uring_cmd_complete_in_task() to cancel command > in ublk_cancel_dev(ublk_cancel_queue). > > Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> > Closes: https://lore.kernel.org/linux-block/becol2g7sawl4rsjq2dztsbc7mqypfqko6wzsyoyazqydoasml@rcxarzwidrhk > Cc: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> Using Ziyang's new blktests test cases, I confirmed this patch avoids the failure I reported. Thanks. Tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
On Thu, May 18, 2023 at 02:01:17AM +0000, Shinichiro Kawasaki wrote: > On May 17, 2023 / 21:34, Ming Lei wrote: > > When handling UBLK_IO_FETCH_REQ, ctx->uring_lock is grabbed first, then > > ub->mutex is acquired. > > > > When handling UBLK_CMD_STOP_DEV or UBLK_CMD_DEL_DEV, ub->mutex is > > grabbed first, then calling io_uring_cmd_done() for canceling uring > > command, in which ctx->uring_lock may be required. > > > > Real deadlock only happens when all the above commands are issued from > > same uring context, and in reality different uring contexts are often used > > for handing control command and IO command. > > > > Fix the issue by using io_uring_cmd_complete_in_task() to cancel command > > in ublk_cancel_dev(ublk_cancel_queue). > > > > Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> > > Closes: https://lore.kernel.org/linux-block/becol2g7sawl4rsjq2dztsbc7mqypfqko6wzsyoyazqydoasml@rcxarzwidrhk > > Cc: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > Using Ziyang's new blktests test cases, I confirmed this patch avoids the > failure I reported. Thanks. > > Tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> Shinichiro, Thanks for the test!
On Wed, 17 May 2023 21:34:08 +0800, Ming Lei wrote: > When handling UBLK_IO_FETCH_REQ, ctx->uring_lock is grabbed first, then > ub->mutex is acquired. > > When handling UBLK_CMD_STOP_DEV or UBLK_CMD_DEL_DEV, ub->mutex is > grabbed first, then calling io_uring_cmd_done() for canceling uring > command, in which ctx->uring_lock may be required. > > [...] Applied, thanks! [1/1] ublk: fix AB-BA lockdep warning commit: ac5902f84bb546c64aea02c439c2579cbf40318f Best regards,
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index c7ed5d69e9ee..33d3298a0da1 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -1120,6 +1120,11 @@ static inline bool ublk_queue_ready(struct ublk_queue *ubq) return ubq->nr_io_ready == ubq->q_depth; } +static void ublk_cmd_cancel_cb(struct io_uring_cmd *cmd, unsigned issue_flags) +{ + io_uring_cmd_done(cmd, UBLK_IO_RES_ABORT, 0, issue_flags); +} + static void ublk_cancel_queue(struct ublk_queue *ubq) { int i; @@ -1131,8 +1136,8 @@ static void ublk_cancel_queue(struct ublk_queue *ubq) struct ublk_io *io = &ubq->ios[i]; if (io->flags & UBLK_IO_FLAG_ACTIVE) - io_uring_cmd_done(io->cmd, UBLK_IO_RES_ABORT, 0, - IO_URING_F_UNLOCKED); + io_uring_cmd_complete_in_task(io->cmd, + ublk_cmd_cancel_cb); } /* all io commands are canceled */
When handling UBLK_IO_FETCH_REQ, ctx->uring_lock is grabbed first, then ub->mutex is acquired. When handling UBLK_CMD_STOP_DEV or UBLK_CMD_DEL_DEV, ub->mutex is grabbed first, then calling io_uring_cmd_done() for canceling uring command, in which ctx->uring_lock may be required. Real deadlock only happens when all the above commands are issued from same uring context, and in reality different uring contexts are often used for handing control command and IO command. Fix the issue by using io_uring_cmd_complete_in_task() to cancel command in ublk_cancel_dev(ublk_cancel_queue). Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> Closes: https://lore.kernel.org/linux-block/becol2g7sawl4rsjq2dztsbc7mqypfqko6wzsyoyazqydoasml@rcxarzwidrhk Cc: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/ublk_drv.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)