All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Ming Lei <ming.lei@redhat.com>
Cc: dm-devel@redhat.com, Alasdair Kergon <agk@redhat.com>,
	Jens Axboe <axboe@fb.com>,
	linux-block@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>,
	Bart Van Assche <bart.vanassche@sandisk.com>,
	Laurence Oberman <loberman@redhat.com>,
	Bart Van Assche <Bart.VanAssche@wdc.com>
Subject: Re: dm-rq: do not update rq partially in each ending bio
Date: Fri, 25 Aug 2017 13:57:22 -0400	[thread overview]
Message-ID: <20170825175722.GA2298@redhat.com> (raw)
In-Reply-To: <20170825170752.GB13937@ming.t460p>

On Fri, Aug 25 2017 at  1:07pm -0400,
Ming Lei <ming.lei@redhat.com> wrote:

> On Fri, Aug 25, 2017 at 12:32:33PM -0400, Mike Snitzer wrote:
> > On Fri, Aug 25 2017 at 12:08pm -0400,
> > Ming Lei <ming.lei@redhat.com> wrote:
> > 
> > > On Fri, Aug 25, 2017 at 11:48:39AM -0400, Mike Snitzer wrote:
> > > > On Fri, Aug 25 2017 at 11:27am -0400,
> > > > Ming Lei <ming.lei@redhat.com> wrote:
> > > > 
> > > > > We don't need to update orignal dm request partially
> > > > > when ending each cloned bio, and this patch just
> > > > > updates orignal dm request once when the whole
> > > > > cloned request is finished.
> > > > > 
> > > > > Partial request update can be a bit expensive, so
> > > > > we should try to avoid it, especially it is run
> > > > > in softirq context.
> > > > > 
> > > > > After this patch is applied, both hard lockup and
> > > > > soft lockup aren't reproduced any more in one hour
> > > > > of running Laurence's test[1] on IB/SRP. Without
> > > > > this patch, the lockup can be reproduced in several
> > > > > minutes.
> > > > > 
> > > > > BTW, after d4acf3650c7c(block: Make blk_mq_delay_kick_requeue_list()
> > > > > rerun the queue at a quiet time), we need to make the
> > > > > test more aggressive for reproducing the lockup:
> > > > > 
> > > > > 	1) run hammer_write.sh 32 or 64 concurrently.
> > > > > 	2) write 8M each time
> > > > > 
> > > > > [1] https://marc.info/?l=linux-block&m=150220185510245&w=2
> > > > 
> > > > Bart said he cannot reproduce the lockups with his patchset applied.
> > > > Have you tested using Bart's patchset?
> > > 
> > > d4acf3650c7c(block: Make blk_mq_delay_kick_requeue_list() rerun the
> > > queue at a quiet time) has been in linus tree.
> > > 
> > > For other patches, I didn't test it yet. Because every time
> > > when the lockup is triggered, it is always in blk_recalc_rq_segments(),
> > > and not see any patch is dealing with that.
> > 
> > Please test with all of Bart's patches applied!
> 
> Just done the test with Bart's patch, still can
> see soft lockup when running the test described
> in commit log for a couple of minutes. BTW, my
> test is much more aggressive than Laurence's, I
> write 8M each time, and run 64 hammer_write.sh
> concurrently.

OK, thanks for verifying as much.
 
> I don't think the two are contradictory. Anyway,
> this patch will decrease CPU utilization of SOFTIRQ, and
> it is a improvement.

OK, looking closer I can see you accomplish the same (retaining partial
completion) in a more optimal way.

I'll include this in my review/staging of dm-multipath patches for 4.14.

Thanks,
Mike

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Ming Lei <ming.lei@redhat.com>
Cc: linux-block@vger.kernel.org,
	Laurence Oberman <loberman@redhat.com>,
	Christoph Hellwig <hch@infradead.org>, Jens Axboe <axboe@fb.com>,
	dm-devel@redhat.com, Bart Van Assche <Bart.VanAssche@wdc.com>,
	Bart Van Assche <bart.vanassche@sandisk.com>,
	Alasdair Kergon <agk@redhat.com>
Subject: Re: dm-rq: do not update rq partially in each ending bio
Date: Fri, 25 Aug 2017 13:57:22 -0400	[thread overview]
Message-ID: <20170825175722.GA2298@redhat.com> (raw)
In-Reply-To: <20170825170752.GB13937@ming.t460p>

On Fri, Aug 25 2017 at  1:07pm -0400,
Ming Lei <ming.lei@redhat.com> wrote:

> On Fri, Aug 25, 2017 at 12:32:33PM -0400, Mike Snitzer wrote:
> > On Fri, Aug 25 2017 at 12:08pm -0400,
> > Ming Lei <ming.lei@redhat.com> wrote:
> > 
> > > On Fri, Aug 25, 2017 at 11:48:39AM -0400, Mike Snitzer wrote:
> > > > On Fri, Aug 25 2017 at 11:27am -0400,
> > > > Ming Lei <ming.lei@redhat.com> wrote:
> > > > 
> > > > > We don't need to update orignal dm request partially
> > > > > when ending each cloned bio, and this patch just
> > > > > updates orignal dm request once when the whole
> > > > > cloned request is finished.
> > > > > 
> > > > > Partial request update can be a bit expensive, so
> > > > > we should try to avoid it, especially it is run
> > > > > in softirq context.
> > > > > 
> > > > > After this patch is applied, both hard lockup and
> > > > > soft lockup aren't reproduced any more in one hour
> > > > > of running Laurence's test[1] on IB/SRP. Without
> > > > > this patch, the lockup can be reproduced in several
> > > > > minutes.
> > > > > 
> > > > > BTW, after d4acf3650c7c(block: Make blk_mq_delay_kick_requeue_list()
> > > > > rerun the queue at a quiet time), we need to make the
> > > > > test more aggressive for reproducing the lockup:
> > > > > 
> > > > > 	1) run hammer_write.sh 32 or 64 concurrently.
> > > > > 	2) write 8M each time
> > > > > 
> > > > > [1] https://marc.info/?l=linux-block&m=150220185510245&w=2
> > > > 
> > > > Bart said he cannot reproduce the lockups with his patchset applied.
> > > > Have you tested using Bart's patchset?
> > > 
> > > d4acf3650c7c(block: Make blk_mq_delay_kick_requeue_list() rerun the
> > > queue at a quiet time) has been in linus tree.
> > > 
> > > For other patches, I didn't test it yet. Because every time
> > > when the lockup is triggered, it is always in blk_recalc_rq_segments(),
> > > and not see any patch is dealing with that.
> > 
> > Please test with all of Bart's patches applied!
> 
> Just done the test with Bart's patch, still can
> see soft lockup when running the test described
> in commit log for a couple of minutes. BTW, my
> test is much more aggressive than Laurence's, I
> write 8M each time, and run 64 hammer_write.sh
> concurrently.

OK, thanks for verifying as much.
 
> I don't think the two are contradictory. Anyway,
> this patch will decrease CPU utilization of SOFTIRQ, and
> it is a improvement.

OK, looking closer I can see you accomplish the same (retaining partial
completion) in a more optimal way.

I'll include this in my review/staging of dm-multipath patches for 4.14.

Thanks,
Mike

  parent reply	other threads:[~2017-08-25 17:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-25 15:27 [PATCH] dm-rq: do not update rq partially in each ending bio Ming Lei
2017-08-25 15:48 ` Mike Snitzer
2017-08-25 16:08   ` Ming Lei
2017-08-25 16:08     ` Ming Lei
2017-08-25 16:32     ` Mike Snitzer
2017-08-25 17:07       ` Ming Lei
2017-08-25 17:14         ` Ming Lei
2017-08-25 17:57         ` Mike Snitzer [this message]
2017-08-25 17:57           ` Mike Snitzer

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=20170825175722.GA2298@redhat.com \
    --to=snitzer@redhat.com \
    --cc=Bart.VanAssche@wdc.com \
    --cc=agk@redhat.com \
    --cc=axboe@fb.com \
    --cc=bart.vanassche@sandisk.com \
    --cc=dm-devel@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=loberman@redhat.com \
    --cc=ming.lei@redhat.com \
    /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.