From: Mike Snitzer <snitzer@redhat.com>
To: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org, dm-devel@redhat.com,
linux-scsi@vger.kernel.org
Subject: Re: [PATCH v4 6/6] dm rq: Avoid that request processing stalls sporadically
Date: Tue, 11 Apr 2017 12:09:58 -0400 [thread overview]
Message-ID: <20170411160958.GA19222@redhat.com> (raw)
In-Reply-To: <20170407181654.27836-7-bart.vanassche@sandisk.com>
On Fri, Apr 07 2017 at 2:16pm -0400,
Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> While running the srp-test software I noticed that request
> processing stalls sporadically at the beginning of a test, namely
> when mkfs is run against a dm-mpath device. Every time when that
> happened the following command was sufficient to resume request
> processing:
>
> echo run >/sys/kernel/debug/block/dm-0/state
>
> This patch avoids that such request processing stalls occur. The
> test I ran is as follows:
>
> while srp-test/run_tests -d -r 30 -t 02-mq; do :; done
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: dm-devel@redhat.com
> ---
> drivers/md/dm-rq.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> index 6886bf160fb2..d19af1d21f4c 100644
> --- a/drivers/md/dm-rq.c
> +++ b/drivers/md/dm-rq.c
> @@ -755,6 +755,7 @@ static int dm_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
> /* Undo dm_start_request() before requeuing */
> rq_end_stats(md, rq);
> rq_completed(md, rq_data_dir(rq), false);
> + blk_mq_delay_run_hw_queue(hctx, 100/*ms*/);
> return BLK_MQ_RQ_QUEUE_BUSY;
> }
>
> --
> 2.12.0
>
I really appreciate your hard work Bart but this looks like a cheap
hack.
I'm clearly too late to stop this from going in (given Jens got it
merged for -rc6) but: this has no place in dm-mq (or any blk-mq
driver). If it is needed it should be elevated to blk-mq core to
trigger blk_mq_delay_run_hw_queue() when BLK_MQ_RQ_QUEUE_BUSY is
returned from blk_mq_ops' .queue_rq.
If this dm-mq specific commit is justified the case certainly is spelled
out in the commit header.
WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
dm-devel@redhat.com
Subject: Re: [PATCH v4 6/6] dm rq: Avoid that request processing stalls sporadically
Date: Tue, 11 Apr 2017 12:09:58 -0400 [thread overview]
Message-ID: <20170411160958.GA19222@redhat.com> (raw)
In-Reply-To: <20170407181654.27836-7-bart.vanassche@sandisk.com>
On Fri, Apr 07 2017 at 2:16pm -0400,
Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> While running the srp-test software I noticed that request
> processing stalls sporadically at the beginning of a test, namely
> when mkfs is run against a dm-mpath device. Every time when that
> happened the following command was sufficient to resume request
> processing:
>
> echo run >/sys/kernel/debug/block/dm-0/state
>
> This patch avoids that such request processing stalls occur. The
> test I ran is as follows:
>
> while srp-test/run_tests -d -r 30 -t 02-mq; do :; done
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: dm-devel@redhat.com
> ---
> drivers/md/dm-rq.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> index 6886bf160fb2..d19af1d21f4c 100644
> --- a/drivers/md/dm-rq.c
> +++ b/drivers/md/dm-rq.c
> @@ -755,6 +755,7 @@ static int dm_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
> /* Undo dm_start_request() before requeuing */
> rq_end_stats(md, rq);
> rq_completed(md, rq_data_dir(rq), false);
> + blk_mq_delay_run_hw_queue(hctx, 100/*ms*/);
> return BLK_MQ_RQ_QUEUE_BUSY;
> }
>
> --
> 2.12.0
>
I really appreciate your hard work Bart but this looks like a cheap
hack.
I'm clearly too late to stop this from going in (given Jens got it
merged for -rc6) but: this has no place in dm-mq (or any blk-mq
driver). If it is needed it should be elevated to blk-mq core to
trigger blk_mq_delay_run_hw_queue() when BLK_MQ_RQ_QUEUE_BUSY is
returned from blk_mq_ops' .queue_rq.
If this dm-mq specific commit is justified the case certainly is spelled
out in the commit header.
next prev parent reply other threads:[~2017-04-11 16:09 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-07 18:16 [PATCH v4 0/6] Avoid that scsi-mq and dm-mq queue processing stalls sporadically Bart Van Assche
2017-04-07 18:16 ` Bart Van Assche
2017-04-07 18:16 ` [PATCH v4 1/6] blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list Bart Van Assche
2017-04-07 18:16 ` Bart Van Assche
2017-04-10 7:10 ` Christoph Hellwig
2017-04-07 18:16 ` [PATCH v4 2/6] blk-mq: Restart a single queue if tag sets are shared Bart Van Assche
2017-04-07 18:16 ` Bart Van Assche
2017-04-10 7:11 ` Christoph Hellwig
2017-04-07 18:16 ` [PATCH v4 3/6] blk-mq: Clarify comments in blk_mq_dispatch_rq_list() Bart Van Assche
2017-04-07 18:16 ` Bart Van Assche
2017-04-10 7:11 ` Christoph Hellwig
2017-04-07 18:16 ` [PATCH v4 4/6] blk-mq: Introduce blk_mq_delay_run_hw_queue() Bart Van Assche
2017-04-07 18:16 ` Bart Van Assche
2017-04-10 7:12 ` Christoph Hellwig
2017-04-10 15:02 ` Jens Axboe
2017-04-07 18:16 ` [PATCH v4 5/6] scsi: Avoid that SCSI queues get stuck Bart Van Assche
2017-04-07 18:16 ` Bart Van Assche
2017-04-10 7:12 ` Christoph Hellwig
2017-04-07 18:16 ` [PATCH v4 6/6] dm rq: Avoid that request processing stalls sporadically Bart Van Assche
2017-04-07 18:16 ` Bart Van Assche
2017-04-11 16:09 ` Mike Snitzer [this message]
2017-04-11 16:09 ` Mike Snitzer
2017-04-11 16:26 ` Bart Van Assche
2017-04-11 16:26 ` Bart Van Assche
2017-04-11 17:47 ` Mike Snitzer
2017-04-11 17:51 ` Bart Van Assche
2017-04-11 17:51 ` Bart Van Assche
2017-04-11 18:03 ` Mike Snitzer
2017-04-11 18:03 ` Mike Snitzer
2017-04-11 18:18 ` Bart Van Assche
2017-04-11 18:18 ` Bart Van Assche
2017-04-12 3:42 ` Ming Lei
2017-04-12 3:42 ` Ming Lei
2017-04-12 18:38 ` Bart Van Assche
2017-04-12 18:38 ` Bart Van Assche
2017-04-12 18:38 ` Bart Van Assche
2017-04-13 2:20 ` Ming Lei
2017-04-13 2:20 ` Ming Lei
2017-04-13 16:59 ` Bart Van Assche
2017-04-14 1:13 ` Ming Lei
2017-04-14 17:12 ` Bart Van Assche
2017-04-14 17:12 ` Bart Van Assche
2017-04-16 10:21 ` Ming Lei
2017-04-07 18:23 ` [PATCH v4 0/6] Avoid that scsi-mq and dm-mq queue " Jens Axboe
2017-04-07 18:33 ` Bart Van Assche
2017-04-07 18:33 ` Bart Van Assche
2017-04-07 18:39 ` Bart Van Assche
2017-04-07 18:39 ` Bart Van Assche
2017-04-07 18:51 ` Jens Axboe
2017-04-12 10:55 ` Benjamin Block
2017-04-12 10:55 ` Benjamin Block
2017-04-12 18:11 ` Bart Van Assche
2017-04-12 18:11 ` Bart Van Assche
2017-04-13 12:23 ` Benjamin Block
2017-04-13 12:23 ` Benjamin Block
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 \
--in-reply-to=20170411160958.GA19222@redhat.com \
--to=snitzer@redhat.com \
--cc=axboe@kernel.dk \
--cc=bart.vanassche@sandisk.com \
--cc=dm-devel@redhat.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.