linux-crypto.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: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	Sultan Alsawaf <sultan@kerneltoast.com>,
	Dominik Brodowski <linux@dominikbrodowski.net>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: [PATCH v2] random: spread out jitter callback to different CPUs
Date: Tue, 29 Nov 2022 17:08:11 +0100	[thread overview]
Message-ID: <20221129160811.563011-1-Jason@zx2c4.com> (raw)
In-Reply-To: <CAHmME9rhtZrwns_d7gQc8vrhWHKZqv6PkWi=JSQXsQOvvRr8UA@mail.gmail.com>

Rather than merely hoping that the callback gets called on another CPU,
arrange for that to actually happen, by round robining which CPU the
timer fires on. This way, on multiprocessor machines, we exacerbate
jitter by touching the same memory from multiple different cores.

There's a little bit of tricky bookkeeping involved here, because using
timer_setup_on_stack() + add_timer_on() + del_timer_sync() is a recipe
for disaster. See this sample code: <https://xn--4db.cc/xBdEiIKO/c>.

Mitigating this by calling del_timer_sync() (or try_to_del_timer_sync())
before each add_timer_on() also isn't satisfactory, because that halts
the main entropy collecting loop while it waits, trying to acquire the
timer base spinlock.

So instead, poll a boolean inside of the main loop to indicate when the
timer callback has finished executing. This serializes the callbacks
while still ensuring the main loop continues.

Cc: Sultan Alsawaf <sultan@kerneltoast.com>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/char/random.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 7b71cea6a6ab..40d0395ea566 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1232,7 +1232,9 @@ void __cold rand_initialize_disk(struct gendisk *disk)
 struct entropy_timer_state {
 	unsigned long entropy;
 	struct timer_list timer;
-	unsigned int samples, samples_per_bit;
+	atomic_t samples;
+	unsigned int samples_per_bit;
+	bool is_running;
 };
 
 /*
@@ -1250,10 +1252,9 @@ static void __cold entropy_timer(struct timer_list *timer)
 {
 	struct entropy_timer_state *state = container_of(timer, struct entropy_timer_state, timer);
 
-	if (++state->samples == state->samples_per_bit) {
+	if (atomic_inc_return(&state->samples) % state->samples_per_bit == 0)
 		credit_init_bits(1);
-		state->samples = 0;
-	}
+	smp_store_release(&state->is_running, false);
 }
 
 /*
@@ -1263,9 +1264,10 @@ static void __cold entropy_timer(struct timer_list *timer)
 static void __cold try_to_generate_entropy(void)
 {
 	enum { NUM_TRIAL_SAMPLES = 8192, MAX_SAMPLES_PER_BIT = HZ / 15 };
-	struct entropy_timer_state stack;
+	struct entropy_timer_state stack = { 0 };
 	unsigned int i, num_different = 0;
 	unsigned long last = random_get_entropy();
+	int cpu = -1;
 
 	for (i = 0; i < NUM_TRIAL_SAMPLES - 1; ++i) {
 		stack.entropy = random_get_entropy();
@@ -1277,19 +1279,38 @@ static void __cold try_to_generate_entropy(void)
 	if (stack.samples_per_bit > MAX_SAMPLES_PER_BIT)
 		return;
 
-	stack.samples = 0;
 	timer_setup_on_stack(&stack.timer, entropy_timer, 0);
 	while (!crng_ready() && !signal_pending(current)) {
-		if (!timer_pending(&stack.timer))
-			mod_timer(&stack.timer, jiffies);
+		/*
+		 * Check both !timer_pending() and state->is_running to ensure that any previous
+		 * callback has finished executing, before queueing the next one.
+		 */
+		if (!timer_pending(&stack.timer) && !smp_load_acquire(&stack.is_running)) {
+			preempt_disable();
+
+			/* Basic CPU round-robin, which avoids the current CPU. */
+			do {
+				cpu = cpumask_next(cpu, cpu_online_mask);
+				if (cpu == nr_cpumask_bits)
+					cpu = cpumask_first(cpu_online_mask);
+			} while (cpu == smp_processor_id() && cpumask_weight(cpu_online_mask) > 1);
+
+			/* Expiring the timer at `jiffies` means it's the next tick. */
+			stack.timer.expires = jiffies;
+
+			smp_store_release(&stack.is_running, true);
+			add_timer_on(&stack.timer, cpu);
+
+			preempt_enable();
+		}
 		mix_pool_bytes(&stack.entropy, sizeof(stack.entropy));
 		schedule();
 		stack.entropy = random_get_entropy();
 	}
+	mix_pool_bytes(&stack.entropy, sizeof(stack.entropy));
 
 	del_timer_sync(&stack.timer);
 	destroy_timer_on_stack(&stack.timer);
-	mix_pool_bytes(&stack.entropy, sizeof(stack.entropy));
 }
 
 
-- 
2.38.1


  reply	other threads:[~2022-11-29 16:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-30 23:10 [PATCH 1/2] random: schedule jitter credit for next jiffy, not in two jiffies Jason A. Donenfeld
2022-09-30 23:10 ` [PATCH 2/2] random: spread out jitter callback to different CPUs Jason A. Donenfeld
2022-10-01  9:21   ` Jason A. Donenfeld
2022-10-05 17:26     ` Sebastian Andrzej Siewior
2022-10-05 21:08       ` Jason A. Donenfeld
2022-10-06  6:46         ` Sebastian Andrzej Siewior
2022-10-06 12:26           ` Jason A. Donenfeld
2022-10-06 12:41             ` Sebastian Andrzej Siewior
2022-10-06 16:39               ` Sultan Alsawaf
2022-10-07  7:29                 ` Sebastian Andrzej Siewior
2022-10-07 14:01             ` Jason A. Donenfeld
2022-10-07 14:55               ` Sebastian Andrzej Siewior
2022-10-07 15:32                 ` Jason A. Donenfeld
2022-11-29 16:08                   ` Jason A. Donenfeld [this message]
2022-11-29 18:27                     ` [PATCH v3] " Jason A. Donenfeld
     [not found]                       ` <20221130014514.6494-1-hdanton@sina.com>
2022-11-30  1:49                         ` Jason A. Donenfeld
2022-11-30 18:48                       ` [PATCH v4] " Jason A. Donenfeld

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=20221129160811.563011-1-Jason@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=bigeasy@linutronix.de \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@dominikbrodowski.net \
    --cc=sultan@kerneltoast.com \
    --cc=tglx@linutronix.de \
    /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).