linux-kernel - Re: [PATCH v3] block: make sure big bio is splitted into at most 256 bvecs

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACVXFVM+RbD47dKsLo-J+aHkkfGjHOmNc03bESnh5H4VOFHgWw@mail.gmail.com>
Date:   Mon, 22 Aug 2016 16:57:42 +0800
From:   Ming Lei <ming.lei@...onical.com>
To:     Kent Overstreet <kent.overstreet@...il.com>
Cc:     Eric Wheeler <bcache@...ts.ewheeler.net>,
        Christoph Hellwig <hch@...radead.org>,
        Jens Axboe <axboe@...com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-block <linux-block@...r.kernel.org>,
        Sebastian Roesner <sroesner-kernelorg@...sner-online.de>,
        "4.3+" <stable@...r.kernel.org>, Shaohua Li <shli@...com>,
        Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH v3] block: make sure big bio is splitted into at most 256 bvecs

On Mon, Aug 22, 2016 at 1:58 AM, Kent Overstreet
<kent.overstreet@...il.com> wrote:
> On Sun, Aug 21, 2016 at 05:31:33PM +0800, Ming Lei wrote:
>> This patch is definitely correct, and I don't see dealing with 'no_merge'
>> should be removed.
>>
>> In this case, the bio is still possible to merge with others because
>> it doesn't violate any limit of the queue because it just can't be held in
>> 256 bvecs, that means it is correct to clear no_merge for this situation.
>>
>> Also I don't think it is complicated or ugly to deal with the flag.
>
> It's extra unnecessary code. All other things being equal, less code is always
> better than more code. It works just fine without it, what's the justification
> for adding the flag?

>From rationality view, the 'no_merge' flag need to be cleared because
it doesn't violate any queue's limit and should be allowed to merge, and
actually it is possible to submit all these splitted bios into hardware via
one request, that is different with other splitting cases.

Given you introduced support of arbitrarily sized bios in commit 54efd50bf
(block:make generic_make_request handle arbitrarily sized bios), other
drivers/fs might use this to simplify bio submission too in the future.

>
> Don't be so dismissive of other maintainer's concerns, we've got to deal with
> this code too. Especially when I and Christoph are agreeing on something - how
> often does that happen?

OK, I will submit a V4 and comment on not merging just for simplicity resaon.


Thanks,
Ming