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>
 <CAHk-=wjLOLWV2NvBPozUj0krF6fvWv6mrC4xpCBVXc=e2+dqPQ@mail.gmail.com> <CANn89i+wBM+ewcP9u+ZWDqv3zQeK7ovKB+YJf9S6Om5QkqhLHA@mail.gmail.com>
In-Reply-To: <CANn89i+wBM+ewcP9u+ZWDqv3zQeK7ovKB+YJf9S6Om5QkqhLHA@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 17 Jun 2022 14:34:59 -0500
Message-ID: <CAHk-=wi9ut1VkB=Ja_gYtH67DZ7cc5QBG-uJCPkOpU=MZDJSUw@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:25 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Interesting...
>
>  I think getrusage(RUSAGE_SELF) is blocking interrupts in the
> possible long loop:

Yeah, that looks bad.

It needs that interrupt disable due to sighand->siglock, but normally
we would expect to *not* have a big loop inside the siglock.

Nasty.

I wonder if this is possibly a situation where we should actually make
siglock be a rwlock.

But considering that this RUSAGE_SELF is hopefully a special case,
maybe we could write it differently.

Instead of taking the sighand lock, we might be able to iterate just
over the regular thread list (using the tasklist lock), and then do
the "does sighand match" as a one-off check in
accumulate_thread_rusage().

It's not like we even really need that strict locking there, I suspect.

Anyway, I should have noted in my previous email that my "rwlock is
often not the win you'd think it is" that that is only true for this
*spinning* rwlock.

For the actual sleeping reader-writer lock (down_read/down_write and
friends), the whole "you can have multiple readers" is often a *huge*
deal and very central to using a rwlock. It's literally just the
spinning one that is often better as a spinlock unless you have those
magical reasons to use it.

              Linus