Re: [PATCH v3 01/14] fs/namei.c: use trailing_slashes()

[David Laight]

On Mon, 6 Jul 2026 22:55:48 +0200 (CEST)
Jori Koolstra <jkoolstra@xxxxxxxxx> wrote:

> > Op 06-07-2026 09:29 CEST schreef David Laight <david.laight.linux@xxxxxxxxx>:
> > 
> >  
> > On Mon, 06 Jul 2026 13:56:00 +1000
> > NeilBrown <neilb@xxxxxxxxxxx> wrote:
> >   
> > > On Sun, 05 Jul 2026, Jori Koolstra wrote:  
> > > > There are several places in fs/namei.c that can use the
> > > > trailing_slashes() function to improve intent.    
> > > 
> > > I think it would help to state here that trailing_slashes() is changed
> > > to take a qstr instead of a nameidata as that is useful in more places.  
> 
> Yes, thanks.
> 
> > 
> > It is also taking the struct 'by value' so should really be always_inline
> > before something makes a compiler decide it should be a real function.
> > 
> > (Or make it take a pointer)
> > 
> > 	David  
> 
> You know more about this compiler optimization stuff than I do, David. So:
> is there any chance this does not get inlined? I am fine with always_inline,
> but I don't see it often in these scenarios (just inline usually). Is there
> a reason for that?

There is a view that you should just let the compiler decide whether to
inline something unless there are technical reasons why it must be inlined.
But I've seen gcc fail to inline static functions that are only called once
even when they are marked 'inline' (that mattered in some embedded code where
any spills to stack would make it too slow).

It is normally the KASAN (etc) options that cause functions not to be inlined
(as well as causing massive stack use when there are lots of calls to functions
that get inlined).

It has been suggested that the kernel be built with inline implying
always_inline - but there are a few quite large functions that are marked
inline and they really don't want to be inlined. 

> 
> Pointer is also OK, but this is just two register copies (even if it does not get
> inlined) on 64-bit. Is that really less fast than one copy and a pointer deref?
> Genuine question, I am no expert on this.

It isn't obvious from the code that the structure is that short (I didn't look
further than the patch and it appears to contain an array).
But passing structures by value is usually either not intended or inefficient.
Any longer than (I think) two registers are passed by taking a copy on stack
and passing that by reference.
(In the pre-ANSI C days you could do that by mistake and spends days trying
to work out why one code path failed to update some members.)

On 32bit I'm pretty sure the three values will be copied and passed
by reference.

This function does have a strange name and implementation.
There is the hidden assumption that if the name isn't '\0' terminated
the extra characters must be '/', and the (bool) cast was probably
added to silence a compiler warning, but '!= 0' would read better.

	David


> 
> Thanks,
> Jori.