All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Bart Van Assche <Bart.VanAssche@wdc.com>
Cc: "ming.lei@redhat.com" <ming.lei@redhat.com>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"hch@infradead.org" <hch@infradead.org>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"axboe@fb.com" <axboe@fb.com>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"jejb@linux.vnet.ibm.com" <jejb@linux.vnet.ibm.com>,
	"loberman@redhat.com" <loberman@redhat.com>,
	"dm-devel@redhat.com" <dm-devel@redhat.com>
Subject: Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE
Date: Tue, 19 Sep 2017 11:56:03 -0400	[thread overview]
Message-ID: <20170919155603.GB22809@redhat.com> (raw)
In-Reply-To: <1505835394.2671.18.camel@wdc.com>

On Tue, Sep 19 2017 at 11:36am -0400,
Bart Van Assche <Bart.VanAssche@wdc.com> wrote:

> On Tue, 2017-09-19 at 13:43 +0800, Ming Lei wrote:
> > On Mon, Sep 18, 2017 at 03:18:16PM +0000, 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 solve? Is it perhaps that there can be a delay between
> > 
> > Actually the improvement on dm-rq IO schedule(the patch 2 ~ 5) doesn't
> > need this patch.
> 
> The approach of this patch series looks wrong to me and patch 5/5 is an ugly
> hack in my opinion. That's why I asked you to drop the entire patch series and
> to test whether inserting a queue run call into the dm-mpath end_io callback
> yields a similar performance improvement to the patches you posted. Please do
> not expect me to spend more time on this patch series if you do not come up
> with measurement results for the proposed alternative.

Bart, asserting that Ming's work is a hack doesn't help your apparent
goal of deligitimizing Ming's work.

Nor does it take away from the fact that your indecision on appropriate
timeouts (let alone ability to defend and/or explain them) validates
Ming calling them into question (which you are now dodging regularly).

But please don't take this feedback and shut-down.  Instead please work
together more constructively.  This doesn't need to be adversarial!  I
am at a loss for why there is such animosity from your end Bart.

Please dial it back.  It is just a distraction that fosters needless
in-fighting.

Believe it or not: Ming is just trying to improve the code because he
has a testbed that is showing fairly abysmal performance with dm-mq
multipath (on lpfc with scsi-mq).

Ming, if you can: please see if what Bart has proposed (instead: run
queue at end_io) helps.  Though I don't yet see why that should be
needed.

Mike

WARNING: multiple messages have this Message-ID (diff)
From: snitzer@redhat.com (Mike Snitzer)
Subject: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE
Date: Tue, 19 Sep 2017 11:56:03 -0400	[thread overview]
Message-ID: <20170919155603.GB22809@redhat.com> (raw)
In-Reply-To: <1505835394.2671.18.camel@wdc.com>

On Tue, Sep 19 2017 at 11:36am -0400,
Bart Van Assche <Bart.VanAssche@wdc.com> wrote:

> On Tue, 2017-09-19@13:43 +0800, Ming Lei wrote:
> > On Mon, Sep 18, 2017@03:18:16PM +0000, 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 solve? Is it perhaps that there can be a delay between
> > 
> > Actually the improvement on dm-rq IO schedule(the patch 2 ~ 5) doesn't
> > need this patch.
> 
> The approach of this patch series looks wrong to me and patch 5/5 is an ugly
> hack in my opinion. That's why I asked you to drop the entire patch series and
> to test whether inserting a queue run call into the dm-mpath end_io callback
> yields a similar performance improvement to the patches you posted. Please do
> not expect me to spend more time on this patch series if you do not come up
> with measurement results for the proposed alternative.

Bart, asserting that Ming's work is a hack doesn't help your apparent
goal of deligitimizing Ming's work.

Nor does it take away from the fact that your indecision on appropriate
timeouts (let alone ability to defend and/or explain them) validates
Ming calling them into question (which you are now dodging regularly).

But please don't take this feedback and shut-down.  Instead please work
together more constructively.  This doesn't need to be adversarial!  I
am at a loss for why there is such animosity from your end Bart.

Please dial it back.  It is just a distraction that fosters needless
in-fighting.

Believe it or not: Ming is just trying to improve the code because he
has a testbed that is showing fairly abysmal performance with dm-mq
multipath (on lpfc with scsi-mq).

Ming, if you can: please see if what Bart has proposed (instead: run
queue at end_io) helps.  Though I don't yet see why that should be
needed.

Mike

  reply	other threads:[~2017-09-19 15:56 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-15 16:44 [PATCH 0/5] dm-mpath: improve I/O schedule Ming Lei
2017-09-15 16:44 ` Ming Lei
2017-09-15 16:44 ` [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE Ming Lei
2017-09-15 16:44   ` Ming Lei
2017-09-15 17:57   ` Bart Van Assche
2017-09-15 17:57     ` Bart Van Assche
2017-09-15 17:57     ` Bart Van Assche
2017-09-17 12:40     ` Ming Lei
2017-09-17 12:40       ` Ming Lei
2017-09-18 15:18       ` Bart Van Assche
2017-09-18 15:18         ` Bart Van Assche
2017-09-18 15:18         ` Bart Van Assche
2017-09-19  5:43         ` Ming Lei
2017-09-19  5:43           ` Ming Lei
2017-09-19 15:36           ` Bart Van Assche
2017-09-19 15:36             ` Bart Van Assche
2017-09-19 15:36             ` Bart Van Assche
2017-09-19 15:56             ` Mike Snitzer [this message]
2017-09-19 15:56               ` Mike Snitzer
2017-09-19 16:04               ` Ming Lei
2017-09-19 16:04                 ` Ming Lei
2017-09-19 16:49                 ` Bart Van Assche
2017-09-19 16:49                   ` Bart Van Assche
2017-09-19 16:49                   ` Bart Van Assche
2017-09-19 16:55                   ` Ming Lei
2017-09-19 16:55                     ` Ming Lei
2017-09-19 18:42                     ` Bart Van Assche
2017-09-19 18:42                       ` Bart Van Assche
2017-09-19 18:42                       ` Bart Van Assche
2017-09-19 22:44                       ` Ming Lei
2017-09-19 22:44                         ` Ming Lei
2017-09-19 23:25                         ` Bart Van Assche
2017-09-19 23:25                           ` Bart Van Assche
2017-09-19 23:25                           ` Bart Van Assche
2017-09-19 23:50                           ` Mike Snitzer
2017-09-19 23:50                             ` Mike Snitzer
2017-09-20  1:13                             ` Ming Lei
2017-09-20  1:13                               ` Ming Lei
2017-09-20  1:19                   ` Ming Lei
2017-09-20  1:19                     ` Ming Lei
2017-09-19 15:48           ` Mike Snitzer
2017-09-19 15:48             ` Mike Snitzer
2017-09-19 15:52             ` Bart Van Assche
2017-09-19 15:52               ` Bart Van Assche
2017-09-19 15:52               ` Bart Van Assche
2017-09-19 16:03               ` Mike Snitzer
2017-09-19 16:03                 ` Mike Snitzer
2017-09-19 16:07             ` Ming Lei
2017-09-19 16:07               ` Ming Lei
2017-09-15 16:44 ` [PATCH 2/5] dm-mpath: return DM_MAPIO_REQUEUE in case of rq allocation failure Ming Lei
2017-09-15 17:29   ` Bart Van Assche
2017-09-15 17:29     ` Bart Van Assche
2017-09-15 20:06     ` Mike Snitzer
2017-09-15 20:48       ` Bart Van Assche
2017-09-15 20:48         ` Bart Van Assche
2017-09-17 13:23       ` Ming Lei
2017-09-19 14:41         ` Mike Snitzer
2017-09-19 15:56           ` Ming Lei
2017-09-17 12:51     ` Ming Lei
2017-09-15 16:44 ` [PATCH 3/5] dm-mpath: remove annoying message of 'blk_get_request() returned -11' Ming Lei
2017-09-15 16:44 ` [PATCH 4/5] block: export blk_update_nr_requests Ming Lei
2017-09-15 16:44 ` [PATCH 5/5] dm-mpath: improve I/O schedule Ming Lei
2017-09-15 20:10   ` Mike Snitzer
2017-09-15 20:56   ` Bart Van Assche
2017-09-15 20:56     ` Bart Van Assche
2017-09-15 21:06   ` Bart Van Assche
2017-09-15 21:06     ` Bart Van Assche
2017-09-15 21:42   ` Bart Van Assche
2017-09-15 21:42     ` Bart Van Assche

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=20170919155603.GB22809@redhat.com \
    --to=snitzer@redhat.com \
    --cc=Bart.VanAssche@wdc.com \
    --cc=axboe@fb.com \
    --cc=dm-devel@redhat.com \
    --cc=hch@infradead.org \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=loberman@redhat.com \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=sagi@grimberg.me \
    /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.