Received-SPF: pass (google.com: domain of linux-kernel+bounces-19741-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 8 Jan 2024 16:12:41 +0100
From: Petr =?UTF-8?B?VGVzYcWZw61r?= <petr@tesarici.cz>
To: Andrew Lunn <andrew@lunn.ch>
Cc: David Laight <David.Laight@aculab.com>, Eric Dumazet
 <edumazet@google.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>,
 Jose Abreu <joabreu@synopsys.com>, "David S. Miller" <davem@davemloft.net>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Maxime
 Coquelin <mcoquelin.stm32@gmail.com>, Chen-Yu Tsai <wens@csie.org>, Jernej
 Skrabec <jernej.skrabec@gmail.com>, Samuel Holland <samuel@sholland.org>,
 "open list:STMMAC ETHERNET DRIVER" <netdev@vger.kernel.org>, "moderated
 list:ARM/STM32 ARCHITECTURE" <linux-stm32@st-md-mailman.stormreply.com>,
 "moderated list:ARM/STM32 ARCHITECTURE"
 <linux-arm-kernel@lists.infradead.org>, open list
 <linux-kernel@vger.kernel.org>, "open list:ARM/Allwinner sunXi SoC support"
 <linux-sunxi@lists.linux.dev>, Jiri Pirko <jiri@resnulli.us>
Subject: Re: [PATCH] net: stmmac: protect statistics updates with a spinlock
Message-ID: <20240108161241.625df103@meshulam.tesarici.cz>
In-Reply-To: <32c4095e-ec33-4059-a8d3-f85e18363c77@lunn.ch>
References: <CANn89iLuYZBersxq4aH-9Fg_ojD0fh=0xtdLbRdbMrup=nvrkA@mail.gmail.com>
	<20240105113402.0f5f1232@meshulam.tesarici.cz>
	<CANn89iLEvW9ZS=+WPETPC=mKRyu9AKmueGCWZZOrz9oX3Xef=g@mail.gmail.com>
	<20240105121447.11ae80d1@meshulam.tesarici.cz>
	<20240105142732.1903bc70@meshulam.tesarici.cz>
	<CANn89iLHLvGFX_JEYU-en0ZoCUpTvjXPBzFECxLFfa_Jhpcjmg@mail.gmail.com>
	<CANn89iKWSemsKmfsLjupwWBnyeKjtHH+mZjTzYiJT4G=xyUrNQ@mail.gmail.com>
	<20240105154558.2ca38aca@meshulam.tesarici.cz>
	<a8bb0eb0-8398-4e7e-8dc5-6ebf2f981ca8@lunn.ch>
	<d05ca29283eb47df9c58838cb87a887c@AcuMS.aculab.com>
	<32c4095e-ec33-4059-a8d3-f85e18363c77@lunn.ch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Mon, 8 Jan 2024 14:41:10 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> > > You might want to consider per CPU statistics. Since each CPU has its
> > > own structure of statistics, you don't need atomic.
> > > 
> > > The code actually using the statistics then needs to sum up the per
> > > CPU statistics, and using syncp should be sufficient for that.  
> > 
> > Doesn't that consume rather a lot of memory on systems with
> > 'silly numbers' of cpu?  
> 
> Systems with silly number of CPUS tend to also have silly amounts of
> memory. We are talking about maybe a dozen u64 here. So the memory
> usage goes from 144 bytes, to 144K for a 1024CPU system.  Is 144K
> excessive for such a system?

I'm not even sure it's worth converting _all_ statistic counters to
per-CPU variables. Most of them are already guarded by a lock (either
the queue lock, or NAPI scheduling). Only the hard interrupt counter is
not protected by anything, so it's more like 8k on a 1024-CPU system....

> > Updating an atomic_t is (pretty much) the same as taking a lock.
> > unlock() is also likely to also contain an atomic operation.
> > So if you update more than two atomic_t it is likely that a lock
> > will be faster.  
> 
> True, but all those 1024 CPUs in your silly system get affected by a
> lock or an atomic. They all need to do something with there L1 and L2
> cache when using atomics. Spending an extra 144K of RAM means the
> other 1023 CPUs don't notice anything at all during the increment
> phase, which could be happening 1M times a second. They only get
> involved when something in user space wants the statistics, so maybe
> once per second from the SNMP agent.
> 
> Also, stmmac is not used on silly CPU systems. It used in embedded
> systems. I doubt its integrated into anything with more than 8 CPUs.

I also doubt it as of today, but hey, it seems that more CPU cores is
the future of embedded. Ten years ago, who would have imagined putting
an 8-core CPU into a smartphone? OTOH who would have imagined a
smartphone with 24G of RAM...

Petr T