From: Ming Lei <[email protected]>
To: Jens Axboe <[email protected]>,
[email protected], [email protected]
Cc: Ming Lei <[email protected]>
Subject: [PATCH for-6.7/io_uring 3/7] ublk: move ublk_cancel_dev() out of ub->mutex
Date: Mon, 9 Oct 2023 17:33:18 +0800 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
ublk_cancel_dev() just calls ublk_cancel_queue() to cancel all pending
io commands after ublk request queue is idle. The only protection is just
the read & write of ubq->nr_io_ready and avoid duplicated command cancel,
so add one per-queue lock with cancel flag for providing this protection,
meantime move ublk_cancel_dev() out of ub->mutex.
Then we needn't to call io_uring_cmd_complete_in_task() to cancel
pending command. And the same cancel logic will be re-used for
cancelable uring command.
This patch basically reverts commit ac5902f84bb5 ("ublk: fix AB-BA lockdep warning").
Signed-off-by: Ming Lei <[email protected]>
---
drivers/block/ublk_drv.c | 40 +++++++++++++++++++++++-----------------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 46d499d96ca3..15b52210488a 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -115,6 +115,9 @@ struct ublk_uring_cmd_pdu {
*/
#define UBLK_IO_FLAG_NEED_GET_DATA 0x08
+/* atomic RW with ubq->cancel_lock */
+#define UBLK_IO_FLAG_CANCELED 0x80000000
+
struct ublk_io {
/* userspace buffer address from io cmd */
__u64 addr;
@@ -139,6 +142,7 @@ struct ublk_queue {
bool force_abort;
bool timeout;
unsigned short nr_io_ready; /* how many ios setup */
+ spinlock_t cancel_lock;
struct ublk_device *dev;
struct ublk_io ios[];
};
@@ -1473,28 +1477,28 @@ 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;
- if (!ublk_queue_ready(ubq))
- return;
-
for (i = 0; i < ubq->q_depth; i++) {
struct ublk_io *io = &ubq->ios[i];
- if (io->flags & UBLK_IO_FLAG_ACTIVE)
- io_uring_cmd_complete_in_task(io->cmd,
- ublk_cmd_cancel_cb);
- }
+ if (io->flags & UBLK_IO_FLAG_ACTIVE) {
+ bool done;
- /* all io commands are canceled */
- ubq->nr_io_ready = 0;
+ spin_lock(&ubq->cancel_lock);
+ done = !!(io->flags & UBLK_IO_FLAG_CANCELED);
+ if (!done)
+ io->flags |= UBLK_IO_FLAG_CANCELED;
+ spin_unlock(&ubq->cancel_lock);
+
+ if (!done)
+ io_uring_cmd_done(io->cmd,
+ UBLK_IO_RES_ABORT, 0,
+ IO_URING_F_UNLOCKED);
+ }
+ }
}
/* Cancel all pending commands, must be called after del_gendisk() returns */
@@ -1541,7 +1545,6 @@ static void __ublk_quiesce_dev(struct ublk_device *ub)
blk_mq_quiesce_queue(ub->ub_disk->queue);
ublk_wait_tagset_rqs_idle(ub);
ub->dev_info.state = UBLK_S_DEV_QUIESCED;
- ublk_cancel_dev(ub);
/* we are going to release task_struct of ubq_daemon and resets
* ->ubq_daemon to NULL. So in monitor_work, check on ubq_daemon causes UAF.
* Besides, monitor_work is not necessary in QUIESCED state since we have
@@ -1564,6 +1567,7 @@ static void ublk_quiesce_work_fn(struct work_struct *work)
__ublk_quiesce_dev(ub);
unlock:
mutex_unlock(&ub->mutex);
+ ublk_cancel_dev(ub);
}
static void ublk_unquiesce_dev(struct ublk_device *ub)
@@ -1603,8 +1607,8 @@ static void ublk_stop_dev(struct ublk_device *ub)
put_disk(ub->ub_disk);
ub->ub_disk = NULL;
unlock:
- ublk_cancel_dev(ub);
mutex_unlock(&ub->mutex);
+ ublk_cancel_dev(ub);
cancel_delayed_work_sync(&ub->monitor_work);
}
@@ -1978,6 +1982,7 @@ static int ublk_init_queue(struct ublk_device *ub, int q_id)
void *ptr;
int size;
+ spin_lock_init(&ubq->cancel_lock);
ubq->flags = ub->dev_info.flags;
ubq->q_id = q_id;
ubq->q_depth = ub->dev_info.queue_depth;
@@ -2585,8 +2590,9 @@ static void ublk_queue_reinit(struct ublk_device *ub, struct ublk_queue *ubq)
int i;
WARN_ON_ONCE(!(ubq->ubq_daemon && ubq_daemon_is_dying(ubq)));
+
/* All old ioucmds have to be completed */
- WARN_ON_ONCE(ubq->nr_io_ready);
+ ubq->nr_io_ready = 0;
/* old daemon is PF_EXITING, put it now */
put_task_struct(ubq->ubq_daemon);
/* We have to reset it to NULL, otherwise ub won't accept new FETCH_REQ */
--
2.41.0
next prev parent reply other threads:[~2023-10-09 9:36 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-09 9:33 [PATCH for-6.7/io_uring 0/7] ublk: simplify abort with cancelable uring_cmd Ming Lei
2023-10-09 9:33 ` [PATCH for-6.7/io_uring 1/7] ublk: don't get ublk device reference in ublk_abort_queue() Ming Lei
2023-10-09 9:33 ` [PATCH for-6.7/io_uring 2/7] ublk: make sure io cmd handled in submitter task context Ming Lei
2023-10-09 9:33 ` Ming Lei [this message]
2023-10-09 9:33 ` [PATCH for-6.7/io_uring 4/7] ublk: rename mm_lock as lock Ming Lei
2023-10-09 9:33 ` [PATCH for-6.7/io_uring 5/7] ublk: quiesce request queue when aborting queue Ming Lei
2023-10-09 9:33 ` [PATCH for-6.7/io_uring 6/7] ublk: replace monitor with cancelable uring_cmd Ming Lei
2023-10-09 9:33 ` [PATCH for-6.7/io_uring 7/7] ublk: simplify aborting request Ming Lei
2023-10-16 23:47 ` [PATCH for-6.7/io_uring 0/7] ublk: simplify abort with cancelable uring_cmd Ming Lei
2023-10-17 1:14 ` Jens Axboe
2023-10-17 1:13 ` Jens Axboe
2023-10-17 5:00 ` Ming Lei
2023-10-17 14:24 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox