linux-kernel - Re: [PATCH] select: do_pollfd: add unlikely branch hint return path

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: <1862386e-fca2-470e-929c-0205a56c0f2f@gmail.com>
Date: Wed, 9 Apr 2025 16:23:28 +0100
From: "Colin King (gmail)" <colin.i.king@...il.com>
To: Mateusz Guzik <mjguzik@...il.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 linux-fsdevel@...r.kernel.org, kernel-janitors@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] select: do_pollfd: add unlikely branch hint return path

On 09/04/2025 16:18, Mateusz Guzik wrote:
> On Wed, Apr 09, 2025 at 03:31:38PM +0100, Colin Ian King wrote:
>> Adding an unlikely() hint on the fd < 0 comparison return path improves
>> run-time performance of the mincore system call. gcov based coverage
>> analysis shows that this path return path is highly unlikely.
>>
>> Benchmarking on an Debian based Intel(R) Core(TM) Ultra 9 285K with
>> a 6.15-rc1 kernel and a poll of 1024 file descriptors with zero timeout
>> shows an call reduction from 32818 ns down to 32635 ns, which is a ~0.5%
>> performance improvement.
>>
>> Results based on running 25 tests with turbo disabled (to reduce clock
>> freq turbo changes), with 30 second run per test and comparing the number
>> of poll() calls per second. The % standard deviation of the 25 tests
>> was 0.08%, so results are reliable.
>>
> 
> I don't think adding a branch hint warrants benchmarking of the sort.
> 
> Instead the thing to do is to check if the prediction matches real world
> uses.
> 
> While it is impossible to check this for all programs out there, it
> should not be a significant time investment to look to check some of the
> popular ones out there. Normally I would do it with bpftrace, but this
> comes from a user-backed area instead of func args, so involved hackery
> may be needed which is not warranted the change. Perhaps running strace
> on a bunch of network progs would also do it (ssh, browser?).
> 
> I have to say I did not even know one can legally pass a fd < 0 to poll
> and I never seen it in action, so I don't expect many users. ;)

I did check this based on gcov coverage (mentioned in the commit 
message) and this is based on running gcov data from running stress-ng 
and kernel builds and I've been looking for branch hint performance wins
based on the top 250 if statements that are not already hinted using 
likely/unlikely.

Colin


Download attachment "OpenPGP_0x68C287DFC6A80226.asc" of type "application/pgp-keys" (4825 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (841 bytes)