netdev - Re: [RFC net-next 1/6] netdevice: Add napi_affinity_no

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

Message-ID: <20240813130556.GD5183@kernel.org>
Date: Tue, 13 Aug 2024 14:05:56 +0100
From: Simon Horman <horms@...nel.org>
To: Joe Damato <jdamato@...tly.com>, Stanislav Fomichev <sdf@...ichev.me>,
	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next 1/6] netdevice: Add napi_affinity_no_change

On Tue, Aug 13, 2024 at 10:11:09AM +0100, Joe Damato wrote:
> On Mon, Aug 12, 2024 at 03:36:42PM -0700, Stanislav Fomichev wrote:
> > On 08/12, Joe Damato wrote:
> > > On Mon, Aug 12, 2024 at 01:23:27PM -0700, Stanislav Fomichev wrote:
> > > > On 08/12, Joe Damato wrote:
> > > > > Several drivers have their own, very similar, implementations of
> > > > > determining if IRQ affinity has changed. Create napi_affinity_no_change
> > > > > to centralize this logic in the core.
> > > > > 
> > > > > This will be used in following commits for various drivers to eliminate
> > > > > duplicated code.
> > > > > 
> 
> [...]
> 
> > > > > +bool napi_affinity_no_change(unsigned int irq)
> > > > > +{
> > > > > +	int cpu_curr = smp_processor_id();
> > > > > +	const struct cpumask *aff_mask;
> > > > > +
> > > > 
> > > > [..]
> > > > 
> > > > > +	aff_mask = irq_get_effective_affinity_mask(irq);
> > > > 
> > > > Most drivers don't seem to call this on every napi_poll (and
> > > > cache the aff_mask somewhere instead). Should we try to keep this
> > > > out of the past path as well?
> > > 
> > > Hm, I see what you mean. It looks like only gve calls it on every
> > > poll, while the others use a cached value.
> > > 
> > > Maybe a better solution is to:
> > >   1. Have the helper take the cached affinity mask from the driver
> > >      and return true/false.
> > >   2. Update gve to cache the mask (like the other 4 are doing).
> > 
> > SG! GVE is definitely the outlier here.
> 
> OK, I'll hack on that for rfcv2 and see what it looks like. Thanks
> for the suggestion.
> 
> Hopefully the maintainers (or other folks) will chime in on whether
> or not I should submit fixes for patches 4 - 6 for the type mismatch
> stuff first or just handle it all together.

<2c>
Patches 4 - 6 seem more like clean-ups that fixes to me: they aren't fixing
any bugs are they? So I would just keep them as part of this patchset
unless it becomes unwieldy.
</2c>

In any case thanks for all your good work in this area.