From: Christoph Hellwig <hch@lst.de>
To: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>,
axboe@fb.com, Matias Bjorling <mb@lightnvm.io>,
linux-block@vger.kernel.org
Subject: Re: [PATCH 01/10] block: don't decrement nr_phys_segments for physically contigous segments
Date: Tue, 14 May 2019 07:14:41 +0200 [thread overview]
Message-ID: <20190514051441.GA6294@lst.de> (raw)
In-Reply-To: <20190514043642.GB10824@ming.t460p>
On Tue, May 14, 2019 at 12:36:43PM +0800, Ming Lei wrote:
> > > Some workloads need this optimization, please see 729204ef49ec00b
> > > ("block: relax check on sg gap"):
> >
> > And we still allow to merge the segments with this patch. The only
> > difference is that these merges do get accounted as extra segments.
>
> It is easy for .nr_phys_segments to reach the max segment limit by this
> way, then no new bio can be merged any more.
As said in my other mail we only decremented it for request merges
in the non-gap case before and no one complained.
> We don't consider segment merge between two bios in ll_new_hw_segment(),
> in my mkfs test over virtio-blk, request size can be increased to ~1M(several
> segments) from 63k(126 bios/segments) easily if the segment merge between
> two bios is considered.
With the gap devices we have unlimited segment size, see my next patch
to actually enforce that. Which is much more efficient than using
multiple segments. Also instead of hacking up the merge path even more
we can fix the block device buffered I/O path to submit large I/Os
instead of relying on merging like we do in the direct I/O code and every
major file system. I have that on my plate as a todo list item.
> > We do that in a couple of places. For one the nvme single segment
> > optimization that triggered this bug. Also for range discard support
> > in nvme and virtio. Then we have loop that iterate the segments, but
> > doesn't use the nr_phys_segments count, and plenty of others that
> > iterate over pages at the moment but should be iterating bvecs,
> > e.g. ubd or aoe.
>
> Seems discard segment doesn't consider bios merge for nvme and virtio,
> so it should be fine in this way. Will take a close look at nvme/virtio
> discard segment merge later.
I found the bio case by looking at doing the proper accounting in the
bio merge path and hitting KASAN warning due to the range kmalloc.
So that issue is real as well.
next prev parent reply other threads:[~2019-05-14 5:15 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-13 6:37 fix nr_phys_segments vs iterators accounting Christoph Hellwig
2019-05-13 6:37 ` [PATCH 01/10] block: don't decrement nr_phys_segments for physically contigous segments Christoph Hellwig
2019-05-13 9:45 ` Ming Lei
2019-05-13 12:03 ` Christoph Hellwig
2019-05-13 12:37 ` Christoph Hellwig
2019-05-14 4:36 ` Ming Lei
2019-05-14 5:14 ` Christoph Hellwig [this message]
2019-05-14 9:05 ` Ming Lei
2019-05-14 13:51 ` Christoph Hellwig
2019-05-14 13:57 ` Hannes Reinecke
2019-05-14 14:27 ` Ming Lei
2019-05-14 14:31 ` Christoph Hellwig
2019-05-14 14:32 ` Christoph Hellwig
2019-05-13 6:37 ` [PATCH 02/10] block: force an unlimited segment size on queues with a virt boundary Christoph Hellwig
2019-05-15 8:19 ` Ming Lei
2019-05-13 6:37 ` [PATCH 03/10] block: remove the segment size check in bio_will_gap Christoph Hellwig
2019-05-15 8:34 ` Ming Lei
2019-05-13 6:37 ` [PATCH 04/10] block: remove the bi_seg_{front,back}_size fields in struct bio Christoph Hellwig
2019-05-13 6:37 ` [PATCH 05/10] block: initialize the write priority in blk_rq_bio_prep Christoph Hellwig
2019-05-13 7:34 ` Minwoo Im
2019-05-13 15:04 ` Chaitanya Kulkarni
2019-05-13 6:37 ` [PATCH 06/10] block: remove blk_init_request_from_bio Christoph Hellwig
2019-05-13 6:37 ` [PATCH 07/10] block: remove the bi_phys_segments field in struct bio Christoph Hellwig
2019-05-13 6:37 ` [PATCH 08/10] block: simplify blk_recalc_rq_segments Christoph Hellwig
2019-05-13 6:37 ` [PATCH 09/10] block: untangle the end of blk_bio_segment_split Christoph Hellwig
2019-05-13 6:37 ` [PATCH 10/10] block: mark blk_rq_bio_prep as inline Christoph Hellwig
2019-05-13 14:57 ` Chaitanya Kulkarni
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=20190514051441.GA6294@lst.de \
--to=hch@lst.de \
--cc=axboe@fb.com \
--cc=linux-block@vger.kernel.org \
--cc=mb@lightnvm.io \
--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.