Hi,
在 2025/7/28 17:28, Jan Kara 写道:
On Sat 26-07-25 02:03:34, Yu Kuai wrote:
From: Yu Kuai <yukuai3@xxxxxxxxxx>
Currently issue io can grab queue_lock three times from bfq_bio_merge(),
bfq_limit_depth() and bfq_prepare_request(), the queue_lock is not
necessary if icq is already created because both queue and ioc can't be
freed before io issuing is done, hence remove the unnecessary queue_lock
and use rcu to protect radix tree lookup.
Noted this is also a prep patch to support request batch dispatching[1].
[1] https://lore.kernel.org/all/20250722072431.610354-1-yukuai1@xxxxxxxxxxxxxxx/
Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx>
Looks good! Just one small comment below. With that fixed feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index ce82770c72ab..ea9c975aaef7 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -308,19 +308,18 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
#ifdef CONFIG_BLK_ICQ
/**
- * ioc_lookup_icq - lookup io_cq from ioc
+ * ioc_lookup_icq - lookup io_cq from ioc in io issue path
* @q: the associated request_queue
*
* Look up io_cq associated with @ioc - @q pair from @ioc. Must be called
- * with @q->queue_lock held.
+ * from io issue path, either return NULL if current issue io to @q for the
+ * first time, or return a valid icq.
*/
struct io_cq *ioc_lookup_icq(struct request_queue *q)
{
struct io_context *ioc = current->io_context;
struct io_cq *icq;
- lockdep_assert_held(&q->queue_lock);
-
/*
* icq's are indexed from @ioc using radix tree and hint pointer,
* both of which are protected with RCU. All removals are done
In this comment there's still reference to holding 'q lock'. I think you
should replace that with justification why when called from IO issue path
we are guaranteed found pointer is safe to use.
Thanks for the review! How about:
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index ea9c975aaef7..fd77e655544f 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -322,9 +322,9 @@ struct io_cq *ioc_lookup_icq(struct request_queue *q)
/*
* icq's are indexed from @ioc using radix tree and hint pointer,
- * both of which are protected with RCU. All removals are done
- * holding both q and ioc locks, and we're holding q lock - if we
- * find a icq which points to us, it's guaranteed to be valid.
+ * both of which are protected with RCU, io issue path ensures that
+ * both request_queue and current task are valid, the founded icq
+ * is guaranteed to be valid until the io is done.
*/
rcu_read_lock();
icq = rcu_dereference(ioc->icq_hint);
Honza
