Skip to content

Commit 748bc4d

Browse files
committed
random: use expired timer rather than wq for mixing fast pool
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 <[email protected]> Tested-by: Paul Webb <[email protected]> Cc: Sherry Yang <[email protected]> Cc: Phillip Goerl <[email protected]> Cc: Jack Vogel <[email protected]> Cc: Nicky Veitch <[email protected]> Cc: Colm Harrington <[email protected]> Cc: Ramanan Govindarajan <[email protected]> Cc: Sebastian Andrzej Siewior <[email protected]> Cc: Dominik Brodowski <[email protected]> Cc: Tejun Heo <[email protected]> Cc: Sultan Alsawaf <[email protected]> Cc: [email protected] Fixes: 58340f8 ("random: defer fast pool mixing to worker") Signed-off-by: Jason A. Donenfeld <[email protected]>
1 parent 9ee0507 commit 748bc4d

File tree

1 file changed

+11
-7
lines changed

1 file changed

+11
-7
lines changed

drivers/char/random.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -921,17 +921,20 @@ struct fast_pool {
921921
unsigned long pool[4];
922922
unsigned long last;
923923
unsigned int count;
924-
struct work_struct mix;
924+
struct timer_list mix;
925925
};
926926

927+
static void mix_interrupt_randomness(struct timer_list *work);
928+
927929
static DEFINE_PER_CPU(struct fast_pool, irq_randomness) = {
928930
#ifdef CONFIG_64BIT
929931
#define FASTMIX_PERM SIPHASH_PERMUTATION
930-
.pool = { SIPHASH_CONST_0, SIPHASH_CONST_1, SIPHASH_CONST_2, SIPHASH_CONST_3 }
932+
.pool = { SIPHASH_CONST_0, SIPHASH_CONST_1, SIPHASH_CONST_2, SIPHASH_CONST_3 },
931933
#else
932934
#define FASTMIX_PERM HSIPHASH_PERMUTATION
933-
.pool = { HSIPHASH_CONST_0, HSIPHASH_CONST_1, HSIPHASH_CONST_2, HSIPHASH_CONST_3 }
935+
.pool = { HSIPHASH_CONST_0, HSIPHASH_CONST_1, HSIPHASH_CONST_2, HSIPHASH_CONST_3 },
934936
#endif
937+
.mix = __TIMER_INITIALIZER(mix_interrupt_randomness, 0)
935938
};
936939

937940
/*
@@ -973,7 +976,7 @@ int __cold random_online_cpu(unsigned int cpu)
973976
}
974977
#endif
975978

976-
static void mix_interrupt_randomness(struct work_struct *work)
979+
static void mix_interrupt_randomness(struct timer_list *work)
977980
{
978981
struct fast_pool *fast_pool = container_of(work, struct fast_pool, mix);
979982
/*
@@ -1027,10 +1030,11 @@ void add_interrupt_randomness(int irq)
10271030
if (new_count < 1024 && !time_is_before_jiffies(fast_pool->last + HZ))
10281031
return;
10291032

1030-
if (unlikely(!fast_pool->mix.func))
1031-
INIT_WORK(&fast_pool->mix, mix_interrupt_randomness);
10321033
fast_pool->count |= MIX_INFLIGHT;
1033-
queue_work_on(raw_smp_processor_id(), system_highpri_wq, &fast_pool->mix);
1034+
if (!timer_pending(&fast_pool->mix)) {
1035+
fast_pool->mix.expires = jiffies;
1036+
add_timer_on(&fast_pool->mix, raw_smp_processor_id());
1037+
}
10341038
}
10351039
EXPORT_SYMBOL_GPL(add_interrupt_randomness);
10361040

0 commit comments

Comments
 (0)