From: Ming Lei <ming.lei@redhat.com>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: Jens Axboe <axboe@fb.com>, Christoph Hellwig <hch@infradead.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Kent Overstreet <kent.overstreet@gmail.com>,
Huang Ying <ying.huang@intel.com>,
linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
Theodore Ts'o <tytso@mit.edu>,
"Darrick J . Wong" <darrick.wong@oracle.com>,
Coly Li <colyli@suse.de>, Filipe Manana <fdmanana@gmail.com>,
Ulf Hansson <ulf.hansson@linaro.org>,
linux-mmc@vger.kernel.org
Subject: Re: [PATCH V4 13/45] block: blk-merge: try to make front segments in full size
Date: Wed, 10 Jan 2018 10:40:53 +0800 [thread overview]
Message-ID: <20180110024046.GA18922@ming.t460p> (raw)
In-Reply-To: <be7f7e1b-88c8-54f5-37c3-672f8426c6ca@gmail.com>
On Tue, Jan 09, 2018 at 08:02:53PM +0300, Dmitry Osipenko wrote:
> On 09.01.2018 17:33, Ming Lei wrote:
> > On Tue, Jan 09, 2018 at 04:18:39PM +0300, Dmitry Osipenko wrote:
> >> On 09.01.2018 05:34, Ming Lei wrote:
> >>> On Tue, Jan 09, 2018 at 12:09:27AM +0300, Dmitry Osipenko wrote:
> >>>> On 18.12.2017 15:22, Ming Lei wrote:
> >>>>> When merging one bvec into segment, if the bvec is too big
> >>>>> to merge, current policy is to move the whole bvec into another
> >>>>> new segment.
> >>>>>
> >>>>> This patchset changes the policy into trying to maximize size of
> >>>>> front segments, that means in above situation, part of bvec
> >>>>> is merged into current segment, and the remainder is put
> >>>>> into next segment.
> >>>>>
> >>>>> This patch prepares for support multipage bvec because
> >>>>> it can be quite common to see this case and we should try
> >>>>> to make front segments in full size.
> >>>>>
> >>>>> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> >>>>> ---
> >>>>> block/blk-merge.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> >>>>> 1 file changed, 49 insertions(+), 5 deletions(-)
> >>>>>
> >>>>> diff --git a/block/blk-merge.c b/block/blk-merge.c
> >>>>> index a476337a8ff4..42ceb89bc566 100644
> >>>>> --- a/block/blk-merge.c
> >>>>> +++ b/block/blk-merge.c
> >>>>> @@ -109,6 +109,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
> >>>>> bool do_split = true;
> >>>>> struct bio *new = NULL;
> >>>>> const unsigned max_sectors = get_max_io_size(q, bio);
> >>>>> + unsigned advance = 0;
> >>>>>
> >>>>> bio_for_each_segment(bv, bio, iter) {
> >>>>> /*
> >>>>> @@ -134,12 +135,32 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
> >>>>> }
> >>>>>
> >>>>> if (bvprvp && blk_queue_cluster(q)) {
> >>>>> - if (seg_size + bv.bv_len > queue_max_segment_size(q))
> >>>>> - goto new_segment;
> >>>>> if (!BIOVEC_PHYS_MERGEABLE(bvprvp, &bv))
> >>>>> goto new_segment;
> >>>>> if (!BIOVEC_SEG_BOUNDARY(q, bvprvp, &bv))
> >>>>> goto new_segment;
> >>>>> + if (seg_size + bv.bv_len > queue_max_segment_size(q)) {
> >>>>> + /*
> >>>>> + * On assumption is that initial value of
> >>>>> + * @seg_size(equals to bv.bv_len) won't be
> >>>>> + * bigger than max segment size, but will
> >>>>> + * becomes false after multipage bvec comes.
> >>>>> + */
> >>>>> + advance = queue_max_segment_size(q) - seg_size;
> >>>>> +
> >>>>> + if (advance > 0) {
> >>>>> + seg_size += advance;
> >>>>> + sectors += advance >> 9;
> >>>>> + bv.bv_len -= advance;
> >>>>> + bv.bv_offset += advance;
> >>>>> + }
> >>>>> +
> >>>>> + /*
> >>>>> + * Still need to put remainder of current
> >>>>> + * bvec into a new segment.
> >>>>> + */
> >>>>> + goto new_segment;
> >>>>> + }
> >>>>>
> >>>>> seg_size += bv.bv_len;
> >>>>> bvprv = bv;
> >>>>> @@ -161,6 +182,12 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
> >>>>> seg_size = bv.bv_len;
> >>>>> sectors += bv.bv_len >> 9;
> >>>>>
> >>>>> + /* restore the bvec for iterator */
> >>>>> + if (advance) {
> >>>>> + bv.bv_len += advance;
> >>>>> + bv.bv_offset -= advance;
> >>>>> + advance = 0;
> >>>>> + }
> >>>>> }
> >>>>>
> >>>>> do_split = false;
> >>>>> @@ -361,16 +388,29 @@ __blk_segment_map_sg(struct request_queue *q, struct bio_vec *bvec,
> >>>>> {
> >>>>>
> >>>>> int nbytes = bvec->bv_len;
> >>>>> + unsigned advance = 0;
> >>>>>
> >>>>> if (*sg && *cluster) {
> >>>>> - if ((*sg)->length + nbytes > queue_max_segment_size(q))
> >>>>> - goto new_segment;
> >>>>> -
> >>>>> if (!BIOVEC_PHYS_MERGEABLE(bvprv, bvec))
> >>>>> goto new_segment;
> >>>>> if (!BIOVEC_SEG_BOUNDARY(q, bvprv, bvec))
> >>>>> goto new_segment;
> >>>>>
> >>>>> + /*
> >>>>> + * try best to merge part of the bvec into previous
> >>>>> + * segment and follow same policy with
> >>>>> + * blk_bio_segment_split()
> >>>>> + */
> >>>>> + if ((*sg)->length + nbytes > queue_max_segment_size(q)) {
> >>>>> + advance = queue_max_segment_size(q) - (*sg)->length;
> >>>>> + if (advance) {
> >>>>> + (*sg)->length += advance;
> >>>>> + bvec->bv_offset += advance;
> >>>>> + bvec->bv_len -= advance;
> >>>>> + }
> >>>>> + goto new_segment;
> >>>>> + }
> >>>>> +
> >>>>> (*sg)->length += nbytes;
> >>>>> } else {
> >>>>> new_segment:
> >>>>> @@ -393,6 +433,10 @@ __blk_segment_map_sg(struct request_queue *q, struct bio_vec *bvec,
> >>>>>
> >>>>> sg_set_page(*sg, bvec->bv_page, nbytes, bvec->bv_offset);
> >>>>> (*nsegs)++;
> >>>>> +
> >>>>> + /* for making iterator happy */
> >>>>> + bvec->bv_offset -= advance;
> >>>>> + bvec->bv_len += advance;
> >>>>> }
> >>>>> *bvprv = *bvec;
> >>>>> }
> >>>>>
> >>>>
> >>>> Hello,
> >>>>
> >>>> This patch breaks MMC on next-20180108, in particular MMC doesn't work anymore
> >>>> with this patch on NVIDIA Tegra20:
> >>>>
> >>>> <3>[ 36.622253] print_req_error: I/O error, dev mmcblk1, sector 512
> >>>> <3>[ 36.671233] print_req_error: I/O error, dev mmcblk2, sector 128
> >>>> <3>[ 36.711308] print_req_error: I/O error, dev mmcblk1, sector 31325304
> >>>> <3>[ 36.749232] print_req_error: I/O error, dev mmcblk2, sector 512
> >>>> <3>[ 36.761235] print_req_error: I/O error, dev mmcblk1, sector 31325816
> >>>> <3>[ 36.832039] print_req_error: I/O error, dev mmcblk2, sector 31259768
> >>>> <3>[ 99.793248] print_req_error: I/O error, dev mmcblk1, sector 31323136
> >>>> <3>[ 99.982043] print_req_error: I/O error, dev mmcblk1, sector 929792
> >>>> <3>[ 99.986301] print_req_error: I/O error, dev mmcblk1, sector 930816
> >>>> <3>[ 100.293624] print_req_error: I/O error, dev mmcblk1, sector 932864
> >>>> <3>[ 100.466839] print_req_error: I/O error, dev mmcblk1, sector 947200
> >>>> <3>[ 100.642955] print_req_error: I/O error, dev mmcblk1, sector 949248
> >>>> <3>[ 100.818838] print_req_error: I/O error, dev mmcblk1, sector 230400
> >>>>
> >>>> Any attempt of mounting MMC block dev ends with a kernel crash. Reverting this
> >>>> patch fixes the issue.
> >>>
> >>> Hi Dmitry,
> >>>
> >>> Thanks for your report!
> >>>
> >>> Could you share us what the segment limits are on your MMC?
> >>>
> >>> cat /sys/block/mmcN/queue/max_segment_size
> >>> cat /sys/block/mmcN/queue/max_segments
> >>>
> >>> Please test the following patch to see if your issue can be fixed?
> >>>
> >>> ---
> >>> diff --git a/block/blk-merge.c b/block/blk-merge.c
> >>> index 446f63e076aa..cfab36c26608 100644
> >>> --- a/block/blk-merge.c
> >>> +++ b/block/blk-merge.c
> >>> @@ -431,12 +431,14 @@ __blk_segment_map_sg(struct request_queue *q, struct bio_vec *bvec,
> >>>
> >>> sg_set_page(*sg, bvec->bv_page, nbytes, bvec->bv_offset);
> >>> (*nsegs)++;
> >>> + }
> >>>
> >>> + *bvprv = *bvec;
> >>> + if (advance) {
> >>> /* for making iterator happy */
> >>> bvec->bv_offset -= advance;
> >>> bvec->bv_len += advance;
> >>> }
> >>> - *bvprv = *bvec;
> >>> }
> >>>
> >>> static inline int __blk_bvec_map_sg(struct request_queue *q, struct bio_vec bv,
> >>
> >> Hi Ming,
> >>
> >> I've tried your patch and unfortunately it doesn't help with the issue.
> >>
> >> Here are the segment limits:
> >>
> >> # cat /sys/block/mmc*/queue/max_segment_size
> >> 65535
> >
> > Hi Dmitry,
> >
> > The 'max_segment_size' of 65535 should be the reason, could you test the
> > following patch?
> >
> > ---
> > diff --git a/block/blk-merge.c b/block/blk-merge.c
> > index 446f63e076aa..38a66e3e678e 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -12,6 +12,8 @@
> >
> > #include "blk.h"
> >
> > +#define sector_align(x) ALIGN_DOWN(x, 512)
> > +
> > static struct bio *blk_bio_discard_split(struct request_queue *q,
> > struct bio *bio,
> > struct bio_set *bs,
> > @@ -109,7 +111,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
> > bool do_split = true;
> > struct bio *new = NULL;
> > const unsigned max_sectors = get_max_io_size(q, bio);
> > - unsigned advance = 0;
> > + int advance = 0;
> >
> > bio_for_each_segment(bv, bio, iter) {
> > /*
> > @@ -144,8 +146,9 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
> > * bigger than max segment size, but this
> > * becomes false after multipage bvecs.
> > */
> > - advance = queue_max_segment_size(q) - seg_size;
> > -
> > + advance = sector_align(
> > + queue_max_segment_size(q) -
> > + seg_size);
> > if (advance > 0) {
> > seg_size += advance;
> > sectors += advance >> 9;
> > @@ -386,7 +389,7 @@ __blk_segment_map_sg(struct request_queue *q, struct bio_vec *bvec,
> > {
> >
> > int nbytes = bvec->bv_len;
> > - unsigned advance = 0;
> > + int advance = 0;
> >
> > if (*sg && *cluster) {
> > if (!BIOVEC_PHYS_MERGEABLE(bvprv, bvec))
> > @@ -400,8 +403,9 @@ __blk_segment_map_sg(struct request_queue *q, struct bio_vec *bvec,
> > * blk_bio_segment_split()
> > */
> > if ((*sg)->length + nbytes > queue_max_segment_size(q)) {
> > - advance = queue_max_segment_size(q) - (*sg)->length;
> > - if (advance) {
> > + advance = sector_align(queue_max_segment_size(q) -
> > + (*sg)->length);
> > + if (advance > 0) {
> > (*sg)->length += advance;
> > bvec->bv_offset += advance;
> > bvec->bv_len -= advance;
> > @@ -431,12 +435,14 @@ __blk_segment_map_sg(struct request_queue *q, struct bio_vec *bvec,
> >
> > sg_set_page(*sg, bvec->bv_page, nbytes, bvec->bv_offset);
> > (*nsegs)++;
> > + }
> >
> > + *bvprv = *bvec;
> > + if (advance > 0) {
> > /* for making iterator happy */
> > bvec->bv_offset -= advance;
> > bvec->bv_len += advance;
> > }
> > - *bvprv = *bvec;
> > }
> >
> > static inline int __blk_bvec_map_sg(struct request_queue *q, struct bio_vec bv,
>
> This patch doesn't help either.
OK, I will send a revert later.
Thinking of the patch further, we don't need this kind of logic for
multipage bvec at all since almost all bvecs won't be contiguous if
bio_add_page() is used after multipage bvec is enabled.
Thanks,
Ming
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2018-01-10 2:41 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-18 12:22 [PATCH V4 00/45] block: support multipage bvec Ming Lei
2017-12-18 12:22 ` [PATCH V4 01/45] block: introduce bio helpers for converting to " Ming Lei
2017-12-18 12:22 ` [PATCH V4 02/45] block: conver to bio_first_bvec_all & bio_first_page_all Ming Lei
2017-12-18 12:22 ` [PATCH V4 03/45] fs: convert to bio_last_bvec_all() Ming Lei
2017-12-18 12:22 ` [PATCH V4 04/45] block: bounce: avoid direct access to bvec table Ming Lei
2017-12-18 12:22 ` [PATCH V4 05/45] block: bounce: don't access bio->bi_io_vec in copy_to_high_bio_irq Ming Lei
2017-12-18 12:22 ` [PATCH V4 06/45] dm: limit the max bio size as BIO_MAX_PAGES * PAGE_SIZE Ming Lei
2017-12-18 12:22 ` [PATCH V4 07/45] bcache: comment on direct access to bvec table Ming Lei
2017-12-18 12:22 ` [PATCH V4 08/45] block: move bio_alloc_pages() to bcache Ming Lei
2017-12-18 12:22 ` [PATCH V4 09/45] btrfs: avoid access to .bi_vcnt directly Ming Lei
2017-12-18 12:22 ` [PATCH V4 10/45] btrfs: avoid to access bvec table directly for a cloned bio Ming Lei
2017-12-18 12:22 ` [PATCH V4 11/45] dm-crypt: don't clear bvec->bv_page in crypt_free_buffer_pages() Ming Lei
2017-12-18 12:22 ` [PATCH V4 12/45] blk-merge: compute bio->bi_seg_front_size efficiently Ming Lei
2017-12-18 12:22 ` [PATCH V4 13/45] block: blk-merge: try to make front segments in full size Ming Lei
2018-01-08 21:09 ` Dmitry Osipenko
2018-01-09 2:34 ` Ming Lei
2018-01-09 13:18 ` Dmitry Osipenko
2018-01-09 14:33 ` Ming Lei
2018-01-09 17:02 ` Dmitry Osipenko
2018-01-10 2:40 ` Ming Lei [this message]
2018-01-10 15:41 ` Dmitry Osipenko
2017-12-18 12:22 ` [PATCH V4 14/45] block: blk-merge: remove unnecessary check Ming Lei
2017-12-18 12:22 ` [PATCH V4 15/45] block: rename bio_for_each_segment* with bio_for_each_page* Ming Lei
2018-01-06 16:19 ` Jens Axboe
2017-12-18 12:22 ` [PATCH V4 16/45] block: rename rq_for_each_segment as rq_for_each_page Ming Lei
2017-12-18 12:22 ` [PATCH V4 17/45] block: rename bio_segments() with bio_pages() Ming Lei
2017-12-18 12:22 ` [PATCH V4 18/45] block: introduce multipage page bvec helpers Ming Lei
2017-12-18 12:22 ` [PATCH V4 19/45] block: introduce bio_for_each_segment() Ming Lei
2017-12-18 12:22 ` [PATCH V4 20/45] block: use bio_for_each_segment() to compute segments count Ming Lei
2017-12-18 12:22 ` [PATCH V4 21/45] block: use bio_for_each_segment() to map sg Ming Lei
2017-12-18 12:22 ` [PATCH V4 22/45] block: introduce segment_last_page() Ming Lei
2017-12-18 12:22 ` [PATCH V4 23/45] fs/buffer.c: use bvec iterator to truncate the bio Ming Lei
2017-12-18 12:22 ` [PATCH V4 24/45] btrfs: use segment_last_page to get bio's last page Ming Lei
2017-12-18 12:22 ` [PATCH V4 25/45] block: implement bio_pages_all() via bio_for_each_page_all() Ming Lei
2017-12-18 12:22 ` [PATCH V4 26/45] block: introduce bio_segments() Ming Lei
2017-12-18 12:22 ` [PATCH V4 27/45] block: introduce rq_for_each_segment() Ming Lei
2017-12-18 12:22 ` [PATCH V4 28/45] block: loop: pass segments to iov_iter Ming Lei
2017-12-18 12:22 ` [PATCH V4 29/45] block: bio: introduce bio_for_each_page_all2 and bio_for_each_segment_all Ming Lei
2017-12-18 12:22 ` [PATCH V4 30/45] block: deal with dirtying pages for multipage bvec Ming Lei
2017-12-18 12:22 ` [PATCH V4 31/45] block: convert to bio_for_each_page_all2() Ming Lei
2017-12-18 12:22 ` [PATCH V4 32/45] md/dm/bcache: conver to bio_for_each_page_all2 and bio_for_each_segment Ming Lei
2017-12-18 12:22 ` [PATCH V4 33/45] fs: conver to bio_for_each_page_all2 Ming Lei
2017-12-18 12:22 ` [PATCH V4 34/45] btrfs: " Ming Lei
2017-12-18 12:22 ` [PATCH V4 35/45] ext4: " Ming Lei
2017-12-18 12:22 ` [PATCH V4 36/45] f2fs: " Ming Lei
2017-12-18 12:22 ` [PATCH V4 37/45] xfs: " Ming Lei
2017-12-18 12:22 ` [PATCH V4 38/45] exofs: " Ming Lei
2017-12-18 12:22 ` [PATCH V4 39/45] gfs2: " Ming Lei
2017-12-18 12:22 ` [PATCH V4 40/45] block: kill bio_for_each_page_all() Ming Lei
2017-12-18 12:22 ` [PATCH V4 41/45] block: rename bio_for_each_page_all2 as bio_for_each_page_all Ming Lei
2017-12-18 12:22 ` [PATCH V4 42/45] block: enable multipage bvecs Ming Lei
2017-12-18 12:22 ` [PATCH V4 43/45] block: bio: pass segments to bio if bio_add_page() is bypassed Ming Lei
2017-12-18 12:22 ` [PATCH V4 44/45] block: always define BIO_MAX_PAGES as 256 Ming Lei
2017-12-18 12:22 ` [PATCH V4 45/45] block: document usage of bio iterator helpers Ming Lei
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=20180110024046.GA18922@ming.t460p \
--to=ming.lei@redhat.com \
--cc=axboe@fb.com \
--cc=colyli@suse.de \
--cc=darrick.wong@oracle.com \
--cc=digetx@gmail.com \
--cc=fdmanana@gmail.com \
--cc=hch@infradead.org \
--cc=kent.overstreet@gmail.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-mmc@vger.kernel.org \
--cc=tytso@mit.edu \
--cc=ulf.hansson@linaro.org \
--cc=viro@zeniv.linux.org.uk \
--cc=ying.huang@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox