Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken

From: Ming Lei
Date: Sat Mar 12 2016 - 05:37:21 EST


Hi Kent,

On Sat, Mar 12, 2016 at 5:24 PM, Kent Overstreet
<kent.overstreet@xxxxxxxxx> wrote:
> On Sat, Mar 12, 2016 at 04:49:41PM +0800, Ming Lei wrote:
>> On Sat, Mar 12, 2016 at 3:43 PM, Kent Overstreet
>> <kent.overstreet@xxxxxxxxx> wrote:
>> > I don't know exactly how it's broken, but with that patch segment counting is
>> > broken - I'm seeing blk_rq_map_sg() overrun the end of the sgtable.
>> >
>> > I suggest reverting it for 4.5...
>>
>> Kent, could you share your test case? I'd like to figure out the root cause.
>
> xfstest 036 on bcachefs.

Given bcachefs isn't merged, could you provide one way to reproduce it
with clean upstream kernel?

>
>> BTW, I don't object to revert it given it is close to v4.5 release, but I am
>> curious how it breaks segment couting.
>
> If you want to debug your version (personally I'd just revert to the simpler
> one), I'd start by having your helper use both methods to calculate the last
> biovec, and then assert that they're equal.
>
> Also make sure you're testing with a sub-page sized blocksize, if filesystem
> blocksize == page size you're not going to be testing the interesting cases

I just run xfstests 036 over bcache and md, with block size 1024/2048, with
xfs/ext4/btrfs, looks the segment counting issue can't be reproduced.

If the issue can only be reproduced with bcachefs, I suggest we don't revert
it until the root cause is figured out.

Thanks,
Ming Lei