Re: [PATCH] err.h: use __always_inline on all error pointer helpers

[David Laight]

On Wed, 27 May 2026 16:25:41 +0200
"Arnd Bergmann" <arnd@xxxxxxxx> wrote:

> On Wed, May 27, 2026, at 16:06, Alexander Lobakin wrote:
> > From: Arnd Bergmann <arnd@xxxxxxxx>
> > Date: Tue, 26 May 2026 23:03:50 +0200  
> >> 
> >> Without CONFIG_PROFILE_ANNOTATED_BRANCHES, the changes are
> >> very small, with around 100 functions growing or shrinking
> >> by a few bytes.
> >> 
> >> I don't think we care much about the size increase when that
> >> option is enabled, but I do wonder what behavior makes more  
> >
> > Yup, and even without this option, __always_inline is better here
> > regardless of how it affects the size. Such oneliners must be
> > transparent to the compiler  
> 
> In general I would trust the compiler to make the right
> choices here, but as I have shown it makes very little difference.
> 
> I think one case where an out-of-line copy may legitimately
> be generated by the compiler would be when optimizing known
> cold code for size and the compiler can show that the
> out of line version is indeed shorter.
> 
> >> sense regarding the annotation for every single IS_ERR(). 
> >> Does it make sense to have every instance get its own counter,
> >> or would it make sense to actually try to reduce these
> >> when profiling the annotations?  
> >
> > I'm not familiar with branch annotations, but from the stats above, it
> > really looks like it adds a lot of code bloat. Plenty of branches in
> > the kernel are sorta pointless to track (the ones which trigger once
> > in a thousand years, the unlikely() ones etc.), I guess.  
> 
> Yes, the CONFIG_PROFILE_ANNOTATED_BRANCHES option definitely
> adds a huge amount of bloat. The point here is to find
> incorrect annotations, either a branch that is marked unlikely()
> but taken most of the time or the reverse. I think
> Steven Rosted enables the option occasionally to
> see if there are any outliers, but nobody should use
> this in production environments.
> 
> For IS_ERR(), it is fairly clear that unlikely() is the
> correct annotation in almost all cases, and it's helpful to
> mark all of the error handling as unlikely so the compiuler
> can move it away from hot code paths. With 35000 instances
> of IS_ERR() there are likely a few exceptions to this
> rule, but I don't know if any of them are important enough
> to require a code change. Steven might remember if he's
> ever seen one here.

IS_ERR_OR_NULL() is more interesting, see https://godbolt.org/z/z3b1Yxqe9

The last one ((unsigned long)p - 1 >= -MAX_ERRNO - 1) only contains
a single branch.

I'm sure I remember Linus ranting about something similar.

-- David

> 
>      Arnd
>