This is a note to let you know that I've just added the patch titled
random: use expired timer rather than wq for mixing fast pool
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
random-use-expired-timer-rather-than-wq-for-mixing-fast-pool.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From 748bc4dd9e663f23448d8ad7e58c011a67ea1eca Mon Sep 17 00:00:00 2001
From: "Jason A. Donenfeld" <Jason@xxxxxxxxx>
Date: Thu, 22 Sep 2022 18:46:04 +0200
Subject: random: use expired timer rather than wq for mixing fast pool
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Jason A. Donenfeld <Jason@xxxxxxxxx>
commit 748bc4dd9e663f23448d8ad7e58c011a67ea1eca upstream.
Previously, the fast pool was dumped into the main pool periodically in
the fast pool's hard IRQ handler. This worked fine and there weren't
problems with it, until RT came around. Since RT converts spinlocks into
sleeping locks, problems cropped up. Rather than switching to raw
spinlocks, the RT developers preferred we make the transformation from
originally doing:
do_some_stuff()
spin_lock()
do_some_other_stuff()
spin_unlock()
to doing:
do_some_stuff()
queue_work_on(some_other_stuff_worker)
This is an ordinary pattern done all over the kernel. However, Sherry
noticed a 10% performance regression in qperf TCP over a 40gbps
InfiniBand card. Quoting her message:
> MT27500 Family [ConnectX-3] cards:
> Infiniband device 'mlx4_0' port 1 status:
> default gid: fe80:0000:0000:0000:0010:e000:0178:9eb1
> base lid: 0x6
> sm lid: 0x1
> state: 4: ACTIVE
> phys state: 5: LinkUp
> rate: 40 Gb/sec (4X QDR)
> link_layer: InfiniBand
>
> Cards are configured with IP addresses on private subnet for IPoIB
> performance testing.
> Regression identified in this bug is in TCP latency in this stack as reported
> by qperf tcp_lat metric:
>
> We have one system listen as a qperf server:
> [root@yourQperfServer ~]# qperf
>
> Have the other system connect to qperf server as a client (in this
> case, it’s X7 server with Mellanox card):
> [root@yourQperfClient ~]# numactl -m0 -N0 qperf 20.20.20.101 -v -uu -ub --time 60 --wait_server 20 -oo msg_size:4K:1024K:*2 tcp_lat
Rather than incur the scheduling latency from queue_work_on, we can
instead switch to running on the next timer tick, on the same core. This
also batches things a bit more -- once per jiffy -- which is okay now
that mix_interrupt_randomness() can credit multiple bits at once.
Reported-by: Sherry Yang <sherry.yang@xxxxxxxxxx>
Tested-by: Paul Webb <paul.x.webb@xxxxxxxxxx>
Cc: Sherry Yang <sherry.yang@xxxxxxxxxx>
Cc: Phillip Goerl <phillip.goerl@xxxxxxxxxx>
Cc: Jack Vogel <jack.vogel@xxxxxxxxxx>
Cc: Nicky Veitch <nicky.veitch@xxxxxxxxxx>
Cc: Colm Harrington <colm.harrington@xxxxxxxxxx>
Cc: Ramanan Govindarajan <ramanan.govindarajan@xxxxxxxxxx>
Cc: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
Cc: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx>
Cc: Tejun Heo <tj@xxxxxxxxxx>
Cc: Sultan Alsawaf <sultan@xxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Fixes: 58340f8e952b ("random: defer fast pool mixing to worker")
Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/char/random.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -894,7 +894,7 @@ struct fast_pool {
unsigned long pool[4];
unsigned long last;
unsigned int count;
- struct work_struct mix;
+ struct timer_list mix;
};
static DEFINE_PER_CPU(struct fast_pool, irq_randomness) = {
@@ -946,9 +946,9 @@ int __cold random_online_cpu(unsigned in
}
#endif
-static void mix_interrupt_randomness(struct work_struct *work)
+static void mix_interrupt_randomness(unsigned long data)
{
- struct fast_pool *fast_pool = container_of(work, struct fast_pool, mix);
+ struct fast_pool *fast_pool = (struct fast_pool *)data;
/*
* The size of the copied stack pool is explicitly 2 longs so that we
* only ever ingest half of the siphash output each time, retaining
@@ -1000,10 +1000,14 @@ void add_interrupt_randomness(int irq)
if (new_count < 1024 && !time_is_before_jiffies(fast_pool->last + HZ))
return;
- if (unlikely(!fast_pool->mix.func))
- INIT_WORK(&fast_pool->mix, mix_interrupt_randomness);
+ if (unlikely(!fast_pool->mix.data))
+ setup_timer(&fast_pool->mix, mix_interrupt_randomness, (unsigned long)fast_pool);
+
fast_pool->count |= MIX_INFLIGHT;
- queue_work_on(raw_smp_processor_id(), system_highpri_wq, &fast_pool->mix);
+ if (!timer_pending(&fast_pool->mix)) {
+ fast_pool->mix.expires = jiffies;
+ add_timer_on(&fast_pool->mix, raw_smp_processor_id());
+ }
}
EXPORT_SYMBOL_GPL(add_interrupt_randomness);
Patches currently in stable-queue which might be from Jason@xxxxxxxxx are
queue-4.9/random-avoid-reading-two-cache-lines-on-irq-randomness.patch
queue-4.9/random-restore-o_nonblock-support.patch
queue-4.9/random-clamp-credited-irq-bits-to-maximum-mixed.patch
queue-4.9/random-use-expired-timer-rather-than-wq-for-mixing-fast-pool.patch
