linux-kernel - Re: [PATCH-tip v6 01/20] locking/rwsem: Prevent decrement of reader count before increment

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]

Date:   Sun, 28 Apr 2019 15:40:01 -0400
From:   Waiman Long <longman@...hat.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Will Deacon <will.deacon@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        huang ying <huang.ying.caritas@...il.com>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH-tip v6 01/20] locking/rwsem: Prevent decrement of reader
 count before increment

On 4/28/19 1:59 PM, Linus Torvalds wrote:
> On Sun, Apr 28, 2019 at 10:41 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>> It's the *first* loop that you could play games with, because you hold
>> the lock, and the list is stable during that loop. So the *first* loop
>> could just walk the list, and then do one list splitting operation
>> instead of doing that "list_move_tail()" thing for each entry.
> .. having looked at that, I would suggest against it.
>
> I _think_ this short and sweet code snippet might just work fine for
> the first loop:
>
>         list_for_each_entry(waiter, &sem->wait_list, list) {
>                 if (waiter->type == RWSEM_WAITING_FOR_WRITE)
>                         break;
>                 woken++;
>         }
>         list_cut_before(&wlist, &sem->wait_list, waiter);
>
> and if it *does* work it would be both smaller and more efficient. But
> it looks a bit too subtle to my taste. Somebody would need to go
> through that with a fine comb, and double-check that it gets the
> "whole list" case right, for example.
>
> So the "phase 1" loop could be perhaps simplified to the above cute things.
>
> But the "phase 2" loop absolutely has to be changed to use
> list_for_each_entry_safe().
>
>                Linus

You are right. Thanks for checking my code. I will make the necessary
change.

Cheers,
Longman