linux-kernel - RE: [PATCH RFC] block: fix bio merge checks when virt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <MWHPR03MB266949004D8BB804718A0258BF9D0@MWHPR03MB2669.namprd03.prod.outlook.com>
Date:   Thu, 15 Dec 2016 14:03:36 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Ming Lei <tom.leiming@...il.com>
CC:     Keith Busch <keith.busch@...el.com>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jens Axboe <axboe@...nel.dk>,
        Dan Williams <dan.j.williams@...el.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Sagi Grimberg <sagig@...lanox.com>,
        Long Li <longli@...rosoft.com>,
        Mike Snitzer <snitzer@...hat.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Cathy Avery <cavery@...hat.com>
Subject: RE: [PATCH RFC] block: fix bio merge checks when virt_boundary is set

> From: linux-kernel-owner@...r.kernel.org [mailto:linux-kernel-
> owner@...r.kernel.org] On Behalf Of Vitaly Kuznetsov
> Sent: Wednesday, April 20, 2016 21:48
> To: Ming Lei <tom.leiming@...il.com>
> Cc: Keith Busch <keith.busch@...el.com>; linux-block@...r.kernel.org; Linux
> Kernel Mailing List <linux-kernel@...r.kernel.org>; Jens Axboe
> <axboe@...nel.dk>; Dan Williams <dan.j.williams@...el.com>; Martin K.
> Petersen <martin.petersen@...cle.com>; Sagi Grimberg
> <sagig@...lanox.com>; Mike Snitzer <snitzer@...hat.com>; KY Srinivasan
> <kys@...rosoft.com>; Cathy Avery <cavery@...hat.com>
> Subject: Re: [PATCH RFC] block: fix bio merge checks when virt_boundary is set
> 
> Ming Lei <tom.leiming@...il.com> writes:
> 
> > On Fri, Mar 18, 2016 at 10:59 AM, Ming Lei <tom.leiming@...il.com> wrote:
> >> On Fri, Mar 18, 2016 at 12:39 AM, Keith Busch <keith.busch@...el.com>
> wrote:
> >>> On Thu, Mar 17, 2016 at 12:20:28PM +0100, Vitaly Kuznetsov wrote:
> >>>> Keith Busch <keith.busch@...el.com> writes:
> >>>> > been combined. In any case, I think you can get what you're after just
> >>>> > by moving the gap check after BIOVEC_PHYS_MERGABLE. Does the
> following
> >>>> > look ok to you?
> >>>> >
> >>>>
> >>>> Thanks, it does.
> >>>
> >>> Cool, thanks for confirming.
> >>>
> >>>> Will you send it or would you like me to do that with your Suggested-by?
> >>>
> >>> I'm not confident yet this doesn't break anything, particularly since
> >>> we moved the gap check after the length check. Just wanted to confirm
> >>> the concept addressed your concern, but still need to take a closer look
> >>> and test before submitting.
> >>
> >> IMO, the change on blk_bio_segment_split() is correct, because actually it
> >> is a sg gap and the check should have been done between segments
> >> instead of bvecs. So it is reasonable to move the check just before populating
> >> a new segment.
> >
> > Thinking of the 1st part change further, looks it is just correct in concept,
> > but wrong from current implementation. Because of bios/reqs merge,
> > blk_rq_map_sg() may end one segment in any bvec in theroy, so I guess
> > that is why each non-1st bvec need the check to make sure no sg gap.
> > Looks a very crazy limit, :-)
> >
> >>
> >> But for the 2nd change in bio_will_gap(), which should fix Vitaly's problem, I
> >> am still not sure if it is completely correct. bio_will_gap() is used
> >> to check if two
> >> bios may be merged. Suppose two bios are continues physically, the last bvec
> >> in 1st bio and the first bvec in 2nd bio might not be in one same segment
> >> because of segment size limit.
> >
> > How about the attached patch?
> >
> 
> I just wanted to revive the discussion as the issue persists. I
> re-tested your patch against 4.6-rc4 and it efficiently solves the
> issue.
> 
> pre-patch:
> # time mkfs.ntfs /dev/sdb1
> Cluster size has been automatically set to 4096 bytes.
> Initializing device with zeroes: 100% - Done.
> Creating NTFS volume structures.
> mkntfs completed successfully. Have a nice day.
> 
> real8m10.977s
> user0m0.115s
> sys0m12.672s
> 
> post-patch:
> # time mkfs.ntfs /dev/sdb1
> Cluster size has been automatically set to 4096 bytes.
> Initializing device with zeroes: 100% - Done.
> Creating NTFS volume structures.
> mkntfs completed successfully. Have a nice day.
> 
> real0m42.430s
> user0m0.171s
> sys0m7.675s
> 
> Will you send this patch? Please let me know if I can further
> assist. Thanks!
> 
> --
>   Vitaly

Hi, I'm reviving the thread because I'm suffering from exactly the same issue.
This is the thread I created today: 
"Big I/O requests are split into small ones due to unaligned ext4 partition boundary?"
http://marc.info/?t=148180346100002&r=1&w=2

Ming's patch can fix this issue for me. 

Stable 4.4 and later are affected too.
I didn't check 4.3.x kernels, but for Linux guest on Hyper-V, any kernel with the
patch "storvsc: get rid of bounce buffer"
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=81988a0e6b031bc80da15257201810ddcf989e64
should be affected.

Thanks,
-- Dexuan