linux-kernel - Re: [PATCH v2] random: use immediate per-cpu timer rather than workqueue for mixing fast pool

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YzKy+bNedt2vu+a1@zx2c4.com>
Date:   Tue, 27 Sep 2022 10:23:21 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Sherry Yang <sherry.yang@...cle.com>,
        Paul Webb <paul.x.webb@...cle.com>,
        Phillip Goerl <phillip.goerl@...cle.com>,
        Jack Vogel <jack.vogel@...cle.com>,
        Nicky Veitch <nicky.veitch@...cle.com>,
        Colm Harrington <colm.harrington@...cle.com>,
        Ramanan Govindarajan <ramanan.govindarajan@...cle.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Tejun Heo <tj@...nel.org>,
        Sultan Alsawaf <sultan@...neltoast.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v2] random: use immediate per-cpu timer rather than
 workqueue for mixing fast pool

On Tue, Sep 27, 2022 at 07:41:52AM +0000, David Laight wrote:
> From: Jason A. Donenfeld
> > Sent: 26 September 2022 23:05
> > 
> > Previously, the fast pool was dumped into the main pool peroidically 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@...rQperfServer ~]# qperf
> > >
> > > Have the other system connect to qperf server as a client (in this
> > > case, it’s X7 server with Mellanox card):
> > > [root@...rQperfClient ~]# 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,
> > deferrably so. This also batches things a bit more -- once per jiffy --
> > which is probably okay now that mix_interrupt_randomness() can credit
> > multiple bits at once. It still puts a bit of pressure on fast_mix(),
> > but hopefully that's acceptable.
> 
> I though NOHZ systems didn't take a timer interrupt every 'jiffy'.
> If that is true what actually happens?

The TIMER_DEFERRABLE part of this patch is a mistake; I'm going to make
that 0. However, since expires==jiffies, there's no difference. It's
still undesirable though.

Jason