diff mbox

[1/7] blk-mq: protect completion path with RCU

Message ID 20171216120726.517153-2-tj@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Tejun Heo Dec. 16, 2017, 12:07 p.m. UTC
Currently, blk-mq protects only the issue path with RCU.  This patch
puts the completion path under the same RCU protection.  This will be
used to synchronize issue/completion against timeout by later patches,
which will also add the comments.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/blk-mq.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig Dec. 29, 2017, 10:04 a.m. UTC | #1
Why do you need the srcu protection?  The completion path can never
sleep.

If there is a good reason to keep it please add commment, and
make the srcu variant a separate function only used by drivers that
need it instead of adding the conditional.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Jan. 8, 2018, 5:12 p.m. UTC | #2
Hello, Christoph.

On Fri, Dec 29, 2017 at 02:04:18AM -0800, Christoph Hellwig wrote:
> Why do you need the srcu protection?  The completion path can never
> sleep.
> 
> If there is a good reason to keep it please add commment, and
> make the srcu variant a separate function only used by drivers that
> need it instead of adding the conditional.

It's either that or adding protection against both srcu and rcu in the
counterpart.  It's easier / cleaner to just use single rcu construct
for a given queue.  Will add a comment.  Jens already has a patch
which factors out the conditionals.

Thanks.
diff mbox

Patch

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 1109747..acf4fbb 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -568,11 +568,23 @@  static void __blk_mq_complete_request(struct request *rq)
 void blk_mq_complete_request(struct request *rq)
 {
 	struct request_queue *q = rq->q;
+	struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, rq->mq_ctx->cpu);
+	int srcu_idx;
 
 	if (unlikely(blk_should_fake_timeout(q)))
 		return;
-	if (!blk_mark_rq_complete(rq))
-		__blk_mq_complete_request(rq);
+
+	if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
+		rcu_read_lock();
+		if (!blk_mark_rq_complete(rq))
+			__blk_mq_complete_request(rq);
+		rcu_read_unlock();
+	} else {
+		srcu_idx = srcu_read_lock(hctx->queue_rq_srcu);
+		if (!blk_mark_rq_complete(rq))
+			__blk_mq_complete_request(rq);
+		srcu_read_unlock(hctx->queue_rq_srcu, srcu_idx);
+	}
 }
 EXPORT_SYMBOL(blk_mq_complete_request);