Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19, 2017 at 04:49:15PM +, Bart Van Assche wrote: > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > > Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART > > to do that already. > > Sorry but I disagree. If SCHED_RESTART is set that causes the blk-mq core

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

Hi Mike, On Tue, Sep 19, 2017 at 07:50:06PM -0400, Mike Snitzer wrote: > On Tue, Sep 19 2017 at 7:25pm -0400, > Bart Van Assche wrote: > > > On Wed, 2017-09-20 at 06:44 +0800, Ming Lei wrote: > > > For this issue, it isn't same between SCSI and dm-rq. > > > > > > We

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19 2017 at 7:25pm -0400, Bart Van Assche wrote: > On Wed, 2017-09-20 at 06:44 +0800, Ming Lei wrote: > > For this issue, it isn't same between SCSI and dm-rq. > > > > We don't need to run queue in .end_io of dm, and the theory is > > simple, otherwise it

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Wed, 2017-09-20 at 06:44 +0800, Ming Lei wrote: > For this issue, it isn't same between SCSI and dm-rq. > > We don't need to run queue in .end_io of dm, and the theory is > simple, otherwise it isn't performance issue, and should be I/O hang. > > 1) every dm-rq's request is 1:1 mapped to

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19, 2017 at 06:42:30PM +, Bart Van Assche wrote: > On Wed, 2017-09-20 at 00:55 +0800, Ming Lei wrote: > > On Wed, Sep 20, 2017 at 12:49 AM, Bart Van Assche > > wrote: > > > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > > > > Run queue at end_io is

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Wed, 2017-09-20 at 00:55 +0800, Ming Lei wrote: > On Wed, Sep 20, 2017 at 12:49 AM, Bart Van Assche > wrote: > > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > > > Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART > > > to do that already.

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Wed, Sep 20, 2017 at 12:49 AM, Bart Van Assche wrote: > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: >> Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART >> to do that already. > > Sorry but I disagree. If SCHED_RESTART is set that causes

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART > to do that already. Sorry but I disagree. If SCHED_RESTART is set that causes the blk-mq core to reexamine the software queues and the hctx dispatch list but not the

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19, 2017 at 11:48:23AM -0400, Mike Snitzer wrote: > On Tue, Sep 19 2017 at 1:43am -0400, > Ming Lei wrote: > > > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > > On Sun, 2017-09-17 at 20:40 +0800, Ming Lei wrote: > > > > "if no request has

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19, 2017 at 11:56:03AM -0400, Mike Snitzer wrote: > On Tue, Sep 19 2017 at 11:36am -0400, > Bart Van Assche wrote: > > > On Tue, 2017-09-19 at 13:43 +0800, Ming Lei wrote: > > > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > > > If you

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19 2017 at 11:52am -0400, Bart Van Assche wrote: > On Tue, 2017-09-19 at 11:48 -0400, Mike Snitzer wrote: > > This thread proves that it is definitely brittle to be relying on fixed > > delays like this: > > https://patchwork.kernel.org/patch/9703249/ > >

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19 2017 at 11:36am -0400, Bart Van Assche wrote: > On Tue, 2017-09-19 at 13:43 +0800, Ming Lei wrote: > > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > > If you are still looking at removing the blk_mq_delay_run_hw_queue() calls > > >

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, 2017-09-19 at 11:48 -0400, Mike Snitzer wrote: > This thread proves that it is definitely brittle to be relying on fixed > delays like this: > https://patchwork.kernel.org/patch/9703249/ Hello Mike, Sorry but I think that's a misinterpretation of my patch. I came up with that patch

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, Sep 19 2017 at 1:43am -0400, Ming Lei wrote: > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > On Sun, 2017-09-17 at 20:40 +0800, Ming Lei wrote: > > > "if no request has completed before the delay has expired" can't be a > > > reason to rerun

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Tue, 2017-09-19 at 13:43 +0800, Ming Lei wrote: > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > If you are still looking at removing the blk_mq_delay_run_hw_queue() calls > > then I think you are looking in the wrong direction. What kind of problem > > are you trying to

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > On Sun, 2017-09-17 at 20:40 +0800, Ming Lei wrote: > > "if no request has completed before the delay has expired" can't be a > > reason to rerun the queue, because the queue can still be busy. > > That statement of you shows that

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Sun, 2017-09-17 at 20:40 +0800, Ming Lei wrote: > "if no request has completed before the delay has expired" can't be a > reason to rerun the queue, because the queue can still be busy. That statement of you shows that there are important aspects of the SCSI core and dm-mpath driver that you

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Fri, Sep 15, 2017 at 05:57:31PM +, Bart Van Assche wrote: > On Sat, 2017-09-16 at 00:44 +0800, Ming Lei wrote: > > If .queue_rq() returns BLK_STS_RESOURCE, blk-mq will rerun > > the queue in the three situations: > > > > 1) if BLK_MQ_S_SCHED_RESTART is set > > - queue is rerun after one rq

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

On Sat, 2017-09-16 at 00:44 +0800, Ming Lei wrote: > If .queue_rq() returns BLK_STS_RESOURCE, blk-mq will rerun > the queue in the three situations: > > 1) if BLK_MQ_S_SCHED_RESTART is set > - queue is rerun after one rq is completed, see blk_mq_sched_restart() > which is run from

[PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

If .queue_rq() returns BLK_STS_RESOURCE, blk-mq will rerun the queue in the three situations: 1) if BLK_MQ_S_SCHED_RESTART is set - queue is rerun after one rq is completed, see blk_mq_sched_restart() which is run from blk_mq_free_request() 2) BLK_MQ_S_TAG_WAITING is set - queue is rerun after