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: <CAHmME9pe2BEJV4WiZNHmDmH_XK621Qqr1JCBdgTNZmr4JGBA4w@mail.gmail.com>
 <20220128223548.97807-1-Jason@zx2c4.com> <CAHmME9qtjZX2kVNSQqUsTrZv1cdR8y6n3yZS-RnpVCCzX9okcA@mail.gmail.com>
 <Yf0JlXf3ARsBpL9K@linutronix.de>
In-Reply-To: <Yf0JlXf3ARsBpL9K@linutronix.de>
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
Date:   Fri, 4 Feb 2022 14:42:03 +0100
Message-ID: <CAHmME9r0XxX3LqNLpVeqAjDQ_OVskPf15QOwxtZYy0tb_x_7HQ@mail.gmail.com>
Subject: Re: [PATCH v2] random: remove batched entropy locking
To:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc:     Andy Lutomirski <luto@amacapital.net>,
        Boqun Feng <boqun.feng@gmail.com>,
        Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Waiman Long <longman@redhat.com>,
        Sultan Alsawaf <sultan@kerneltoast.com>,
        "Theodore Ts'o" <tytso@mit.edu>, Andy Lutomirski <luto@kernel.org>,
        =?UTF-8?Q?Jonathan_Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        LKML <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Sebastian,

Please calm down a bit: this patch doesn't minimize the importance of
working out a real solution for PREEMPT_RT, and I'm not under the
illusion that this one here is the silver bullet. It does, however,
have other merits, which may or may not have anything to do with
PREEMPT_RT. To reiterate: I am taking your PREEMPT_RT concerns
seriously, and I want to come up with a solution to that, which we're
working toward more broadly in that other thread.

Per your feedback on v1, this is no longer marked for stable and no
longer purports to fix the PREEMPT_RT issues entirely. Actually, a
large motivation for this includes the reason why Andy's original
patch was laying around in the first place: we're trying to make this
code faster.

I can improve the commit message a bit though.

On Fri, Feb 4, 2022 at 12:10 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> - This splat only occurs with CONFIG_PROVE_RAW_LOCK_NESTING enabled.

Right, the commit message for v2 mentions that.

> - The problem identified by the splat affects only PREEMPT_RT. Non-RT is
>   not affected by this.

Right.

>
> - This patch disables interrupts and invokes extract_crng() which leads
>   to other problems.

The existing code, which uses a spinlock, also disables interrupts,
right? So this isn't actually regressing in that regard. It just
doesn't fix your PREEMPT_RT issue, right?

Or is the issue you see that spinlock_t is a mutex on PREEMPT_RT, so
we're disabling interrupts here in a way that we _weren't_ originally,
in a PREEMPT_RT context? If that's the case, then I think I see your
objection.

I wonder if it'd be enough here to disable preemption instead? But
then we run into trouble if this is called from an interrupt.

Maybe it'd be best to retain the spinlock_t, which will amount to
disabling interrupts on !PREEMPT_RT, since it'll never be contended,
but will turn into a mutex on PREEMPT_RT, where it'll do the right
thing from an exclusivity perspective. Would this be reasonable?

Andy? Any suggestions?

Jason