linux-kernel - Re: [PATCH v3 2/2] pid: only take pidmap

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] [day] [month] [year] [list]

Message-ID: <20251207103755.70be2f89@pumpkin>
Date: Sun, 7 Dec 2025 10:37:55 +0000
From: David Laight <david.laight.linux@...il.com>
To: Mateusz Guzik <mjguzik@...il.com>
Cc: Matthew Wilcox <willy@...radead.org>, oleg@...hat.com,
 brauner@...nel.org, linux-kernel@...r.kernel.org,
 akpm@...ux-foundation.org, linux-mm@...ck.org
Subject: Re: [PATCH v3 2/2] pid: only take pidmap_lock once on alloc

On Sun, 7 Dec 2025 10:21:35 +0100
Mateusz Guzik <mjguzik@...il.com> wrote:

> On Sun, Dec 7, 2025 at 8:21 AM Matthew Wilcox <willy@...radead.org> wrote:
> >
> > On Sat, Dec 06, 2025 at 02:19:55PM +0100, Mateusz Guzik wrote:  
> > > -     if (!pid)
> > > +     if (unlikely(!pid))  
> >
> > Does this change anything?  I was under the impression that gcc already
> > treats !p as unlikely.  
> 
> I don't know if gcc is guaranteed to act like that, most of my
> experience is with clang which was rather inconsistent about it.
> 

If nothing else unlikely() are information for the human (and other) reader.

In my experience, a simple if () is always coded as a forwards jump around
the controlled code - regardless of any likely/unlikely annotation.
Similarly 'if () continue/break' is an immediate jump to the loop end code.
You need to add a non-empty 'else' branch, or something before the
continue/break for the un/likely to have an effect (an asm() comment will do).

While some instruction sets have explicit 'branch expected taken' markers
others (like x86 - except P6) don't.
Similarly some cpu predict backwards branches taken if their is nothing
in the branch predictor, others (including some x86) just use the relevant
branch predictor 'slot' without regard for the actual branch address.
Then there is the branch prediction done by the instruction fetch/decode
unit, that has a tendency to use markers on the cache line - so doesn't
necessarily even check it is a branch instruction!

Basically the coder doesn't have much control at all on many cpu.

Oh, and using un/likely() can make the code worse.

	David