[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <650d8af4.5d0a0220.5ce38.2c5e@mx.google.com>
Date: Fri, 22 Sep 2023 14:39:13 +0200
From: Christian Marangi <ansuelsmth@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Vincent Whitchurch <vincent.whitchurch@...s.com>,
Raju Rangoju <rajur@...lsio.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Ping-Ke Shih <pkshih@...ltek.com>,
Kalle Valo <kvalo@...nel.org>, Simon Horman <horms@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Jiri Pirko <jiri@...nulli.us>,
Hangbin Liu <liuhangbin@...il.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org,
linux-wireless@...r.kernel.org
Subject: Re: [net-next PATCH 3/3] net: stmmac: increase TX coalesce timer to
5ms
On Fri, Sep 22, 2023 at 02:28:06PM +0200, Andrew Lunn wrote:
> On Fri, Sep 22, 2023 at 01:12:47PM +0200, Christian Marangi wrote:
> > Commit 8fce33317023 ("net: stmmac: Rework coalesce timer and fix
> > multi-queue races") decreased the TX coalesce timer from 40ms to 1ms.
> >
> > This caused some performance regression on some target (regression was
> > reported at least on ipq806x) in the order of 600mbps dropping from
> > gigabit handling to only 200mbps.
> >
> > The problem was identified in the TX timer getting armed too much time.
> > While this was fixed and improved in another commit, performance can be
> > improved even further by increasing the timer delay a bit moving from
> > 1ms to 5ms.
> >
> > The value is a good balance between battery saving by prevending too
> > much interrupt to be generated and permitting good performance for
> > internet oriented devices.
>
> ethtool has a settings you can use for this:
>
> ethtool -C|--coalesce devname [adaptive-rx on|off] [adaptive-tx on|off]
> [rx-usecs N] [rx-frames N] [rx-usecs-irq N] [rx-frames-irq N]
> [tx-usecs N] [tx-frames N] [tx-usecs-irq N] [tx-frames-irq N]
> [stats-block-usecs N] [pkt-rate-low N] [rx-usecs-low N]
> [rx-frames-low N] [tx-usecs-low N] [tx-frames-low N]
> [pkt-rate-high N] [rx-usecs-high N] [rx-frames-high N]
> [tx-usecs-high N] [tx-frames-high N] [sample-interval N]
> [cqe-mode-rx on|off] [cqe-mode-tx on|off] [tx-aggr-max-bytes N]
> [tx-aggr-max-frames N] [tx-aggr-time-usecs N]
>
> If this is not implemented, i suggest you add support for it.
>
> Changing the default might cause regressions. Say there is a VoIP
> application which wants this low latency? It would be safer to allow
> user space to configure it as wanted.
>
Yep stmmac already support it. Idea here was to not fallback to use
ethtool and find a good value.
Just for reference before one commit, the value was set to 40ms and
nobody ever pointed out regression about VoIP application. Wtih some
testing I found 5ms a small increase that restore original perf and
should not cause any regression.
(for reference keeping this to 1ms cause a lost of about 100-200mbps)
(also the tx timer implementation was created before any napi poll logic
and before dma interrupt handling was a thing, with the later change I
expect this timer to be very little used in VoIP scenario or similar
with continuous traffic as napi will take care of handling packet)
Aside from these reason I totally get the concern and totally ok with
this not getting applied, was just an idea to push for a common value.
Just preferred to handle this here instead of script+userspace :(
(the important part is the previous patch)
--
Ansuel
Powered by blists - more mailing lists