Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220617091039.2257083-1-eric.dumazet@gmail.com>
 <YqxufxqsnHjVfQOs@worktop.programming.kicks-ass.net> <2dd754f9-3a79-ed17-e423-6b411c3afb69@redhat.com>
 <CALvZod5ijDz=coEE8G8v_haPaKuUa5jHYzEwKvLVxHGphixsFA@mail.gmail.com>
 <2730b855-8f99-5a9e-707e-697d3bd9811d@redhat.com> <CANn89iJLWJMmNrLYQ0EU7_0Wri6c3Kn9vYMOiWu1Ds8Af2KOnw@mail.gmail.com>
 <7499dd05-30d1-669c-66b4-5cb06452b476@redhat.com> <CANn89iLxX_bqD8PvAkZXGWzKBKYxB3qaqQjxxdmoG91PfmvRnA@mail.gmail.com>
 <YqzQKER4JRoudTJE@hirez.programming.kicks-ass.net> <CANn89iKO1koPa5R_mvK0k2dkFaq+F0PgcbvpVt+JpzzR5xsu6g@mail.gmail.com>
In-Reply-To: <CANn89iKO1koPa5R_mvK0k2dkFaq+F0PgcbvpVt+JpzzR5xsu6g@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 17 Jun 2022 14:19:32 -0500
Message-ID: <CAHk-=wjLOLWV2NvBPozUj0krF6fvWv6mrC4xpCBVXc=e2+dqPQ@mail.gmail.com>
Subject: Re: [PATCH] locking/rwlocks: do not starve writers
To:     Eric Dumazet <edumazet@google.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Waiman Long <longman@redhat.com>,
        Shakeel Butt <shakeelb@google.com>,
        Eric Dumazet <eric.dumazet@gmail.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@redhat.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        Will Deacon <will@kernel.org>, Roman Penyaev <rpenyaev@suse.de>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jun 17, 2022 at 2:10 PM Eric Dumazet <edumazet@google.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.

              Linus