Re: Very slow qemu device access

[Ming Lei <ming.lei@redhat.com>]

On Sun, Aug 09, 2020 at 03:25:22PM +0100, Matthew Wilcox wrote:
> On Sun, Aug 09, 2020 at 10:40:05AM +0800, Ming Lei wrote:
> > Hello Matthew,
> > 
> > On Fri, Aug 07, 2020 at 06:44:16PM +0100, Matthew Wilcox wrote:
> > > 
> > > Everything starts going very slowly after this commit:
> > > 
> > > commit 37f4a24c2469a10a4c16c641671bd766e276cf9f (refs/bisect/bad)
> > > Author: Ming Lei <ming.lei@redhat.com>
> > > Date:   Tue Jun 30 22:03:57 2020 +0800
> > > 
> > >     blk-mq: centralise related handling into blk_mq_get_driver_tag
> > 
> > Yeah, the above is one known bad commit, which is reverted in
> > 4e2f62e566b5 ("Revert "blk-mq: put driver tag when this request is completed")
> > 
> > Finally the fixed patch of 'blk-mq: centralise related handling into blk_mq_get_driver_tag'
> > is merged as 568f27006577 ("blk-mq: centralise related handling into blk_mq_get_driver_tag").
> > 
> > So please test either 4e2f62e566b5 or 568f27006577 and see if there is
> > such issue.
> 
> 4e2f62e566b5 is good
> 568f27006577 is bad

Please try the following patch, and we shouldn't take flush request
account into driver tag allocation, because it always shares the
data request's tag:

From d508415eee08940ff9c78efe0eddddf594afdb94 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Mon, 10 Aug 2020 11:06:15 +0800
Subject: [PATCH] block: don't double account of flush request's driver tag

In case of none scheduler, we share data request's driver tag for
flush request, so have to mark the flush request as INFLIGHT for
avoiding double account of this driver tag.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
Fixes: 568f27006577 ("blk-mq: centralise related handling into blk_mq_get_driver_tag")
Reported-by: Matthew Wilcox <willy@infradead.org>
---
 block/blk-flush.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index 6e1543c10493..53abb5c73d99 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -308,9 +308,16 @@ static void blk_kick_flush(struct request_queue *q, struct blk_flush_queue *fq,
 	flush_rq->mq_ctx = first_rq->mq_ctx;
 	flush_rq->mq_hctx = first_rq->mq_hctx;
 
-	if (!q->elevator)
+	if (!q->elevator) {
 		flush_rq->tag = first_rq->tag;
-	else
+
+		/*
+		 * We borrow data request's driver tag, so have to mark
+		 * this flush request as INFLIGHT for avoiding double
+		 * account of this driver tag
+		 */
+		flush_rq->rq_flags |= RQF_MQ_INFLIGHT;
+	} else
 		flush_rq->internal_tag = first_rq->internal_tag;
 
 	flush_rq->cmd_flags = REQ_OP_FLUSH | REQ_PREFLUSH;
-- 
2.25.2

 

thanks,
Ming