From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>,
Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Peter Zijlstra" <peterz@infradead.org>,
"Theodore Ts'o" <tytso@mit.edu>,
"Sultan Alsawaf" <sultan@kerneltoast.com>,
"Jonathan Neuschäfer" <j.neuschaefer@gmx.net>,
"Dominik Brodowski" <linux@dominikbrodowski.net>
Subject: [PATCH v7] random: defer fast pool mixing to worker
Date: Fri, 11 Feb 2022 18:07:32 +0100 [thread overview]
Message-ID: <20220211170732.571775-1-Jason@zx2c4.com> (raw)
In-Reply-To: <CAHmME9rC_q4LGq2JaAAeGbtRA2cibTe9bnvhMLng+QnzAy2DVg@mail.gmail.com>
On PREEMPT_RT, it's problematic to take spinlocks from hard irq
handlers. We can fix this by deferring to a workqueue the dumping of
the fast pool into the input pool.
We accomplish this with some careful rules on fast_pool->count:
- When it's incremented to >= 64, we schedule the work.
- If the top bit is set, we never schedule the work, even if >= 64.
- The worker is responsible for setting it back to 0 when it's done.
There are two small issues around using workqueues for this purpose that
we work around.
The first issue is that mix_interrupt_randomness() might be migrated to
another CPU during CPU hotplug. This issue is rectified by checking that
it hasn't been migrated (after disabling irqs). If it has been migrated,
then we set the count to zero, so that when the CPU comes online again,
it can requeue the work. As part of this, we switch to using an
atomic_t, so that the increment in the irq handler doesn't wipe out the
zeroing if the CPU comes back online while this worker is running.
The second issue is that, though relatively minor in effect, we probably
want to make sure we get a consistent view of the pool onto the stack,
in case it's interrupted by an irq while reading. To do this, we don't
reenable irqs until after the copy. There are only 18 instructions
between the cli and sti, so this is a pretty tiny window.
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Sultan Alsawaf <sultan@kerneltoast.com>
Cc: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Reviewed-by: Dominik Brodowski <linux@dominikbrodowski.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Sebastian - as requested, we now disable irqs for a very short 18
instructions rather than fishing into migrate_disable() and upsetting
PeterZ. Might this be the lucky patch? -Jason
drivers/char/random.c | 63 +++++++++++++++++++++++++++++++++----------
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 1dbf21c02b40..b921127ea693 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1162,9 +1162,10 @@ EXPORT_SYMBOL_GPL(add_bootloader_randomness);
struct fast_pool {
unsigned long pool[16 / sizeof(long)];
+ struct work_struct mix;
unsigned long last;
+ atomic_t count;
u16 reg_idx;
- u8 count;
};
/*
@@ -1214,12 +1215,49 @@ static u32 get_reg(struct fast_pool *f, struct pt_regs *regs)
return *ptr;
}
+static void mix_interrupt_randomness(struct work_struct *work)
+{
+ struct fast_pool *fast_pool = container_of(work, struct fast_pool, mix);
+ unsigned long pool[ARRAY_SIZE(fast_pool->pool)];
+
+ /* Check to see if we're running on the wrong CPU due to hotplug. */
+ local_irq_disable();
+ if (fast_pool != this_cpu_ptr(&irq_randomness)) {
+ local_irq_enable();
+ /*
+ * If we are unlucky enough to have been moved to another CPU,
+ * during CPU hotplug while the CPU was shutdown then we set
+ * our count to zero atomically so that when the CPU comes
+ * back online, it can enqueue work again. The _release here
+ * pairs with the atomic_inc_return_acquire in
+ * add_interrupt_randomness().
+ */
+ atomic_set_release(&fast_pool->count, 0);
+ return;
+ }
+
+ /*
+ * Copy the pool to the stack so that the mixer always has a
+ * consistent view, before we reenable irqs again.
+ */
+ memcpy(pool, fast_pool->pool, sizeof(pool));
+ atomic_set(&fast_pool->count, 0);
+ fast_pool->last = jiffies;
+ local_irq_enable();
+
+ mix_pool_bytes(pool, sizeof(pool));
+ credit_entropy_bits(1);
+ memzero_explicit(pool, sizeof(pool));
+}
+
void add_interrupt_randomness(int irq)
{
+ enum { MIX_INFLIGHT = 1U << 31 };
struct fast_pool *fast_pool = this_cpu_ptr(&irq_randomness);
struct pt_regs *regs = get_irq_regs();
unsigned long now = jiffies;
cycles_t cycles = random_get_entropy();
+ unsigned int new_count;
if (cycles == 0)
cycles = get_reg(fast_pool, regs);
@@ -1235,12 +1273,13 @@ void add_interrupt_randomness(int irq)
}
fast_mix((u32 *)fast_pool->pool);
- ++fast_pool->count;
+ /* The _acquire here pairs with the atomic_set_release in mix_interrupt_randomness(). */
+ new_count = (unsigned int)atomic_inc_return_acquire(&fast_pool->count);
if (unlikely(crng_init == 0)) {
- if (fast_pool->count >= 64 &&
+ if (new_count >= 64 &&
crng_fast_load(fast_pool->pool, sizeof(fast_pool->pool)) > 0) {
- fast_pool->count = 0;
+ atomic_set(&fast_pool->count, 0);
fast_pool->last = now;
/*
@@ -1254,20 +1293,16 @@ void add_interrupt_randomness(int irq)
return;
}
- if ((fast_pool->count < 64) && !time_after(now, fast_pool->last + HZ))
+ if (new_count & MIX_INFLIGHT)
return;
- if (!spin_trylock(&input_pool.lock))
+ if (new_count < 64 && !time_after(now, fast_pool->last + HZ))
return;
- fast_pool->last = now;
- _mix_pool_bytes(&fast_pool->pool, sizeof(fast_pool->pool));
- spin_unlock(&input_pool.lock);
-
- fast_pool->count = 0;
-
- /* Award one bit for the contents of the fast pool. */
- credit_entropy_bits(1);
+ if (unlikely(!fast_pool->mix.func))
+ INIT_WORK(&fast_pool->mix, mix_interrupt_randomness);
+ atomic_or(MIX_INFLIGHT, &fast_pool->count);
+ queue_work_on(raw_smp_processor_id(), system_highpri_wq, &fast_pool->mix);
}
EXPORT_SYMBOL_GPL(add_interrupt_randomness);
--
2.35.0
next prev parent reply other threads:[~2022-02-11 17:07 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-11 13:08 [PATCH v5] random: defer fast pool mixing to worker Jason A. Donenfeld
2022-02-11 15:00 ` Sebastian Andrzej Siewior
2022-02-11 16:25 ` [PATCH v6] " Jason A. Donenfeld
2022-02-11 16:44 ` Sebastian Andrzej Siewior
2022-02-11 16:50 ` Jason A. Donenfeld
2022-02-11 16:58 ` Sebastian Andrzej Siewior
2022-02-11 17:00 ` Jason A. Donenfeld
2022-02-11 17:15 ` Sebastian Andrzej Siewior
2022-02-11 17:17 ` Jason A. Donenfeld
2022-02-11 17:26 ` Sebastian Andrzej Siewior
2022-02-13 21:04 ` Jason A. Donenfeld
2022-02-14 10:19 ` Jason A. Donenfeld
2022-02-13 17:37 ` Jason A. Donenfeld
2022-02-14 9:16 ` Sebastian Andrzej Siewior
2022-02-14 10:17 ` Jason A. Donenfeld
2022-02-14 11:16 ` Sebastian Andrzej Siewior
2022-02-14 14:47 ` Jason A. Donenfeld
2022-02-11 17:07 ` Jason A. Donenfeld [this message]
2022-02-11 17:20 ` [PATCH v7] " Sultan Alsawaf
2022-02-11 17:24 ` Sebastian Andrzej Siewior
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=20220211170732.571775-1-Jason@zx2c4.com \
--to=jason@zx2c4.com \
--cc=bigeasy@linutronix.de \
--cc=j.neuschaefer@gmx.net \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@dominikbrodowski.net \
--cc=peterz@infradead.org \
--cc=sultan@kerneltoast.com \
--cc=tglx@linutronix.de \
--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