Re: [PATCH net-next] netpoll: optimize struct layout for cache efficiency

[Breno Leitao]

On Tue, Mar 25, 2025 at 08:48:38AM -0700, Jakub Kicinski wrote:
> On Mon, 24 Mar 2025 05:29:13 -0700 Breno Leitao wrote:
> > The struct netpoll serves two distinct purposes: it contains
> > configuration data needed only during setup (in netpoll_setup()), and
> > runtime data that's accessed on every packet transmission (in
> > netpoll_send_udp()).
> > 
> > Currently, this structure spans three cache lines with suboptimal
> > organization, where frequently accessed fields are mixed with rarely
> > accessed ones.
> > 
> > This commit reorganizes the structure to place all runtime fields used
> > during packet transmission together in the first cache line, while
> > moving the setup-only configuration fields to subsequent cache lines.
> > This approach follows the principle of placing hot fields together for
> > better cache locality during the performance-critical path.
> > 
> > The restructuring also eliminates structural inefficiencies, reducing
> > the number of holes. This provides a more compact memory layout while
> > maintaining the same functionality, resulting in better cache
> > utilization and potentially improves performance during packet
> > transmission operations.
> 
> Netpoll shouldn't send too many packets, "not too many" for networking
> means >100kpps. So I don't think the hot / close split matters?

I see your point. The gain is going to be marginal given the frequency
this netpoll is supposed to be called, for sure.

On the other side, I think this is still better than the current state,
given: 

 * it has no adverse effect
 * potential marginal performance win
 * structure packing, potentially saving 2 bytes.

> >   -   /* sum members: 137, holes: 3, sum holes: 7 */
> >   +   /* sum members: 137, holes: 1, sum holes: 3 */
> > 
> > Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
> > ---
> >  include/linux/netpoll.h | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
> > index 0477208ed9ffa..a8de41d84be52 100644
> > --- a/include/linux/netpoll.h
> > +++ b/include/linux/netpoll.h
> > @@ -24,7 +24,16 @@ union inet_addr {
> >  
> >  struct netpoll {
> >  	struct net_device *dev;
> > +	u16 local_port, remote_port;
> >  	netdevice_tracker dev_tracker;
> 
> It's a little odd to leave the tracker in hot data, if you do it
> should at least be adjacent to the pointer it tracks?

Double-checking this better, netdevice_tracker is NOT on the hot path.
It is only used on the setup functions.

If you think this is not a total waste of time, I will send a v2 moving
it to the bottom.

Thanks for your review,
--breno