linux-kernel - Re: [PATCH v2] blk: improve order of bio handling in generic_make

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <153a6cff-c553-0d18-e15b-4f3defc3a42b@profitbricks.com>
Date:   Fri, 10 Mar 2017 16:07:58 +0100
From:   Jack Wang <jinpu.wang@...fitbricks.com>
To:     Mikulas Patocka <mpatocka@...hat.com>,
        Mike Snitzer <snitzer@...hat.com>
Cc:     Lars Ellenberg <lars.ellenberg@...bit.com>,
        NeilBrown <neilb@...e.com>, Jens Axboe <axboe@...nel.dk>,
        LKML <linux-kernel@...r.kernel.org>,
        Kent Overstreet <kent.overstreet@...il.com>,
        Pavel Machek <pavel@....cz>, linux-raid@...r.kernel.org,
        device-mapper development <dm-devel@...hat.com>,
        linux-block@...r.kernel.org
Subject: Re: [PATCH v2] blk: improve order of bio handling in
 generic_make_request()



On 10.03.2017 15:55, Mikulas Patocka wrote:
> 
> 
> On Fri, 10 Mar 2017, Mike Snitzer wrote:
> 
>> On Fri, Mar 10 2017 at  7:34am -0500,
>> Lars Ellenberg <lars.ellenberg@...bit.com> wrote:
>>
>>>> --- a/block/blk-core.c
>>>> +++ b/block/blk-core.c
>>>> @@ -1975,7 +1975,14 @@ generic_make_request_checks(struct bio *bio)
>>>>   */
>>>>  blk_qc_t generic_make_request(struct bio *bio)
>>>>  {
>>>> -       struct bio_list bio_list_on_stack;
>>>> +       /*
>>>> +        * bio_list_on_stack[0] contains bios submitted by the current
>>>> +        * make_request_fn.
>>>> +        * bio_list_on_stack[1] contains bios that were submitted before
>>>> +        * the current make_request_fn, but that haven't been processed
>>>> +        * yet.
>>>> +        */
>>>> +       struct bio_list bio_list_on_stack[2];
>>>>         blk_qc_t ret = BLK_QC_T_NONE;
>>>
>>> May I suggest that, if you intend to assign something that is not a
>>> plain &(struct bio_list), but a &(struct bio_list[2]),
>>> you change the task member so it is renamed (current->bio_list vs
>>> current->bio_lists, plural, is what I did last year).
>>> Or you will break external modules, silently, and horribly (or,
>>> rather, they won't notice, but break the kernel).
>>> Examples of such modules would be DRBD, ZFS, quite possibly others.
>>
>> drbd is upstream -- so what is the problem?  (if you are having to
>> distribute drbd independent of the upstream drbd then why is drbd
>> upstream?)
>>
>> As for ZFS, worrying about ZFS kABI breakage is the last thing we should
>> be doing.
> 
> It's better to make external modules not compile than to silently 
> introduce bugs in them. So yes, I would rename that.
> 
> Mikulas

Agree, better rename current->bio_list to current->bio_lists

Regards,
Jack