[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d251f785-407e-d63f-0591-de6251f6b14f@redhat.com>
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
Powered by blists - more mailing lists