linux-kernel - Re: [PATCH] locking/rwlocks: do not starve writers

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: <CALvZod4CTHV=M7SmzByNymLWAJKxXg2YNXg=7gFxOhs37UsSsA@mail.gmail.com>
Date:   Fri, 17 Jun 2022 12:34:40 -0700
From:   Shakeel Butt <shakeelb@...gle.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Waiman Long <longman@...hat.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Will Deacon <will@...nel.org>, Roman Penyaev <rpenyaev@...e.de>
Subject: Re: [PATCH] locking/rwlocks: do not starve writers

On Fri, Jun 17, 2022 at 12:19 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Fri, Jun 17, 2022 at 2:10 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > So I wonder why we replaced eventpoll spinlock with an rwlock.
>
> Yeah, usually we've actually gone the other way.
>
> Spinning rwlocks are seldom a big win, unless you can get some
> secondary indirect win out of them.
>
> That secondary win is often:
>
>  (a) unfairness is usually very good for throughput (iow, the very
> unfairness that you hit may *be* the reason why it looked good in some
> benchmark, and people decided "ok, let's do this").
>
>  (b) the special case of "interrupts take the lock for reading only"
> thing that allows other readers to not disable interrupts
>
> IOW, the win of a spinning rwlock is not necessarily the "we allow
> multiple concurrent readers" that you'd expect, because if you have
> small sections of code you protect, that just isn't a big deal, and
> the costs are in the lock bouncing etc.
>
> It's also worth pointing out that rwlocks are only unfair *if* they
> hit that "reader from (soft)interrupt" case. Which means that such
> cases *really* had better either have very very short locked regions
> (with interrupts disabled), or they really need that (b) part above.
>
> And yes, the tasklist lock really needs the (b) part above. Disabling
> interrupts for task traversal would be completely and entirely
> unacceptable, because the traversal can actually be fairly expensive
> (lots and lots of threads).
>
> I suspect eventpoll just did the wrong thing.
>

In addition the commit log of commit a218cc491420 has this comment:
"(I assume that write side of a rwlock does not starve, it seems
qrwlock implementation has these guarantees)."

Since this assumption seems incorrect, is there any objection in
reverting the commit a218cc491420? Or do we need more
evidence/arguments/repro?