linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org
Cc: herbert@gondor.apana.org.au,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	Theodore Ts'o <tytso@mit.edu>,
	Dominik Brodowski <linux@dominikbrodowski.net>
Subject: [PATCH v2] random: use static branch for crng_ready()
Date: Thu, 12 May 2022 14:22:44 +0200	[thread overview]
Message-ID: <20220512122244.2805-1-Jason@zx2c4.com> (raw)
In-Reply-To: <Ynzs/TDjALqfD9vN@zx2c4.com>

Since crng_ready() is only false briefly during initialization and then
forever after becomes true, we don't need to evaluate it after, making
it a prime candidate for a static branch.

One complication, however, is that it changes state in a particular call
to credit_init_bits(), which might be made from atomic context, which
means we must kick off a workqueue to change the static key.

Further complicating things, credit_init_bits() may be called
sufficiently early on in system initialization such that system_wq is
NULL. In that case, we skip scheduling the work item from
credit_init_bits(), but check to see if the rng is already initialized
by the time it hits rand_initialize(), in which case we assume that
there was no crediting event between system_wq becoming non-NULL and
rand_initialize() being called, so we schedule the work then.

We make that assumption because this is still being called in an early
boot environement in which IRQs are disabled. To understand more
clearly, the two flows are as follows.

First possibility, in which credit_init_bits() is called relatively late
in boot:

  - workqueue_init_early()
  - ...
  - rand_initialize()
    * crng_ready()==false → do nothing
  - ...
  - credit_init_bits(256)
    * system_wq!=NULL → schedule_work()

Second possibility, in which credit_init_bits() is called by the super
early EFI-handling code:

  - credit_init_bits(256)
    * system_wq==NULL → do nothing
  - workqueue_init_early()
  - rcu_init(), trace_init(), initcall_debug_enable(),
    context_tracking_init(), early_irq_init(), init_IRQ(), tick_init(),
    rcu_init_nohz(), init_timers(), srcu_init(), hrtimers_init(),
    softirq_init(), timekeeping_init(), kfence_init(), time_init()
    * We assume that none of these call credit_init_bits(). Since IRQs
      are still off, we don't have to worry about other code preempting
      these.
  - rand_initialize()
    * crng_ready()==true → schedule_work()

Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Dominik - this is somewhat reminiscent of the misery we went through
with the NUMA node workqueue situation, which I was pretty happy about
removing. Hopefully recent other simplifications make adding something
similar back in not quite as bad, but if you've got the bandwidth, this
could use another pair of eyes. -Jason

 drivers/char/random.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 8fa7a5b9aa93..d4bc9beaed2c 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -79,7 +79,8 @@ static enum {
 	CRNG_EARLY = 1, /* At least POOL_EARLY_BITS collected */
 	CRNG_READY = 2  /* Fully initialized with POOL_READY_BITS collected */
 } crng_init = CRNG_EMPTY;
-#define crng_ready() (likely(crng_init >= CRNG_READY))
+static DEFINE_STATIC_KEY_FALSE(crng_is_ready);
+#define crng_ready() (static_branch_likely(&crng_is_ready) || crng_init >= CRNG_READY)
 /* Various types of waiters for crng_init->CRNG_READY transition. */
 static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait);
 static struct fasync_struct *fasync;
@@ -109,6 +110,12 @@ bool rng_is_initialized(void)
 }
 EXPORT_SYMBOL(rng_is_initialized);
 
+static void crng_set_ready(struct work_struct *work)
+{
+	static_branch_enable(&crng_is_ready);
+}
+static DECLARE_WORK(crng_set_ready_work, crng_set_ready);
+
 /* Used by wait_for_random_bytes(), and considered an entropy collector, below. */
 static void try_to_generate_entropy(void);
 
@@ -268,6 +275,8 @@ static void crng_reseed(void)
 		++next_gen;
 	WRITE_ONCE(base_crng.generation, next_gen);
 	WRITE_ONCE(base_crng.birth, jiffies);
+	if (!crng_ready() && system_wq)
+		schedule_work(&crng_set_ready_work);
 	crng_init = CRNG_READY;
 	spin_unlock_irqrestore(&base_crng.lock, flags);
 	memzero_explicit(key, sizeof(key));
@@ -945,9 +954,18 @@ int __init rand_initialize(void)
 	_mix_pool_bytes(&now, sizeof(now));
 	_mix_pool_bytes(utsname(), sizeof(*(utsname())));
 
-	if (crng_ready())
+	if (crng_ready()) {
+		/*
+		 * If this function is called with the crng already
+		 * initialized, then it means it was done so prior to
+		 * system_wq being available, which means we should now
+		 * schedule the work to change the static key from here.
+		 */
+		schedule_work(&crng_set_ready_work);
+
+		/* Immediately use the above architectural contributions. */
 		crng_reseed();
-	else if (arch_init && trust_cpu)
+	} else if (arch_init && trust_cpu)
 		credit_init_bits(BLAKE2S_BLOCK_SIZE * 8);
 
 	WARN_ON(register_pm_notifier(&pm_notifier));
-- 
2.35.1


  reply	other threads:[~2022-05-12 12:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-03 13:40 [PATCH RFC v1] random: use static branch for crng_ready() Jason A. Donenfeld
2022-05-11 11:41 ` Jason A. Donenfeld
2022-05-12  4:35   ` Herbert Xu
2022-05-12 11:18     ` Jason A. Donenfeld
2022-05-12 12:22       ` Jason A. Donenfeld [this message]
2022-05-12 14:42         ` [PATCH v3] " Jason A. Donenfeld
2022-05-12 16:37           ` [PATCH v4] " Jason A. Donenfeld
2022-05-13  6:22             ` Dominik Brodowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220512122244.2805-1-Jason@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@dominikbrodowski.net \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).