Date: Tue, 4 Jun 2024 12:42:43 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Paolo Abeni <pabeni@...hat.com>, Mina Almasry <almasrymina@...gle.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-alpha@...r.kernel.org,
 linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
 sparclinux@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 linux-arch@...r.kernel.org, bpf@...r.kernel.org,
 linux-kselftest@...r.kernel.org, linux-media@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Donald Hunter <donald.hunter@...il.com>, Jonathan Corbet <corbet@....net>,
 Richard Henderson <richard.henderson@...aro.org>, Ivan Kokshaysky
 <ink@...assic.park.msu.ru>, Matt Turner <mattst88@...il.com>, Thomas
 Bogendoerfer <tsbogend@...ha.franken.de>, "James E.J. Bottomley"
 <James.Bottomley@...senpartnership.com>, Helge Deller <deller@....de>,
 Andreas Larsson <andreas@...sler.com>, Jesper Dangaard Brouer
 <hawk@...nel.org>, Ilias Apalodimas <ilias.apalodimas@...aro.org>, Masami
 Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Arnd Bergmann <arnd@...db.de>, Alexei
 Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>, Song Liu
 <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, John Fastabend
 <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Stanislav
 Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Jiri Olsa
 <jolsa@...nel.org>, Steffen Klassert <steffen.klassert@...unet.com>,
 Herbert Xu <herbert@...dor.apana.org.au>, David Ahern <dsahern@...nel.org>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, Shuah Khan
 <shuah@...nel.org>, Sumit Semwal <sumit.semwal@...aro.org>, Christian
 König <christian.koenig@....com>, Pavel Begunkov
 <asml.silence@...il.com>, David Wei <dw@...idwei.uk>, Yunsheng Lin
 <linyunsheng@...wei.com>, Shailend Chand <shailend@...gle.com>, Harshitha
 Ramamurthy <hramamurthy@...gle.com>, Shakeel Butt <shakeel.butt@...ux.dev>,
 Jeroen de Borst <jeroendb@...gle.com>, Praveen Kaligineedi
 <pkaligineedi@...gle.com>, Willem de Bruijn <willemb@...gle.com>, Kaiyuan
 Zhang <kaiyuanz@...gle.com>
Subject: Re: [PATCH net-next v10 05/14] netdev: netdevice devmem allocator

On Tue, 4 Jun 2024 13:31:58 -0300
Jason Gunthorpe <jgg@...pe.ca> wrote:

> On Tue, Jun 04, 2024 at 12:15:51PM -0400, Steven Rostedt wrote:
> > On Tue, 04 Jun 2024 12:13:15 +0200
> > Paolo Abeni <pabeni@...hat.com> wrote:
> >   
> > > On Thu, 2024-05-30 at 20:16 +0000, Mina Almasry wrote:  
> > > > diff --git a/net/core/devmem.c b/net/core/devmem.c
> > > > index d82f92d7cf9ce..d5fac8edf621d 100644
> > > > --- a/net/core/devmem.c
> > > > +++ b/net/core/devmem.c
> > > > @@ -32,6 +32,14 @@ static void net_devmem_dmabuf_free_chunk_owner(struct gen_pool *genpool,
> > > >  	kfree(owner);
> > > >  }
> > > >  
> > > > +static inline dma_addr_t net_devmem_get_dma_addr(const struct net_iov *niov)    
> > > 
> > > Minor nit: please no 'inline' keyword in c files.  
> > 
> > I'm curious. Is this a networking rule? I use 'inline' in my C code all the
> > time.  
> 
> It mostly comes from Documentation/process/coding-style.rst:
> 
> 15) The inline disease
> ----------------------
> 
> There appears to be a common misperception that gcc has a magic "make me
> faster" speedup option called ``inline``. While the use of inlines can be
> appropriate (for example as a means of replacing macros, see Chapter 12), it
> very often is not. Abundant use of the inline keyword leads to a much bigger
> kernel, which in turn slows the system as a whole down, due to a bigger
> icache footprint for the CPU and simply because there is less memory
> available for the pagecache. Just think about it; a pagecache miss causes a
> disk seek, which easily takes 5 milliseconds. There are a LOT of cpu cycles
> that can go into these 5 milliseconds.
> 
> A reasonable rule of thumb is to not put inline at functions that have more
> than 3 lines of code in them. An exception to this rule are the cases where
> a parameter is known to be a compiletime constant, and as a result of this
> constantness you *know* the compiler will be able to optimize most of your
> function away at compile time. For a good example of this later case, see
> the kmalloc() inline function.
> 
> Often people argue that adding inline to functions that are static and used
> only once is always a win since there is no space tradeoff. While this is
> technically correct, gcc is capable of inlining these automatically without
> help, and the maintenance issue of removing the inline when a second user
> appears outweighs the potential value of the hint that tells gcc to do
> something it would have done anyway.
> 

Interesting, as I sped up the ftrace ring buffer by a substantial amount by
adding strategic __always_inline, noinline, likely() and unlikely()
throughout the code. It had to do with what was considered the fast path
and slow path, and not actually the size of the function. gcc got it
horribly wrong.

-- Steve

Powered by blists - more mailing lists