Received-SPF: pass (google.com: domain of linux-crypto-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: <20220915002235.v2.1.I7c0a79e9b3c52584f5b637fde5f1d6f807605806@changeid>
In-Reply-To: <20220915002235.v2.1.I7c0a79e9b3c52584f5b637fde5f1d6f807605806@changeid>
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
Date:   Fri, 16 Sep 2022 15:50:41 +0100
Message-ID: <CAHmME9rhunb05DEnc=UfGr8k9_LBi1NW2Hi0OsRbGwcCN2NzjQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] random: move add_hwgenerator_randomness()'s wait
 outside function
To:     Sven van Ashbrook <svenva@chromium.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Dominik Brodowski <linux@dominikbrodowski.net>,
        Olivia Mackall <olivia@selenic.com>,
        Alex Levin <levinale@google.com>,
        Andrey Pronin <apronin@google.com>,
        Stephen Boyd <swboyd@google.com>,
        Rajat Jain <rajatja@google.com>,
        Eric Biggers <ebiggers@google.com>,
        Petr Mladek <pmladek@suse.com>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        "Theodore Ts'o" <tytso@mit.edu>, linux-crypto@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Sven,

On Thu, Sep 15, 2022 at 1:22 AM Sven van Ashbrook <svenva@chromium.org> wrote:
> -       if (!kthread_should_stop() && crng_ready())
> -               schedule_timeout_interruptible(CRNG_RESEED_INTERVAL);
> +       return crng_ready() ? CRNG_RESEED_INTERVAL : 0;

I was thinking the other day that under certain circumstances, it
would be nice if random.c could ask hwrng for more bytes NOW, rather
than waiting. With the code as it is currently, this could be
accomplished by having a completion event or something similar to
that. With your proposed change, now it's left up to the hwrng
interface to handle.

That's not the end of the world, but it does mean we'd have to come up
with a patch down the line that exports a hwrng function saying, "stop
the delays and schedule the worker NOW". Now impossible, just more
complex, as now the state flow is split across two places. Wondering
if you have any thoughts about this.

The other thing that occurred to me when reading this patch in context
of the other one is that this sleep you're removing here is not the
only sleep in the call chain. Each hwrng driver can also sleep, and
many do, sometimes for a long time, blocking until there's data
available, which might happen after minutes in some cases. So maybe
that's something to think about in context of this patchset -- that
just moving this to a delayed worker might not actually fix the issue
you're having with sleeps.

Jason