Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 15 Apr 2022 15:44:20 +0200
From:   =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= <clement.leger@bootlin.com>
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Magnus Damm <magnus.damm@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Herve Codina <herve.codina@bootlin.com>,
        =?UTF-8?B?TWlxdcOobA==?= Raynal <miquel.raynal@bootlin.com>,
        Milan Stevanovic <milan.stevanovic@se.com>,
        Jimmy Lalande <jimmy.lalande@se.com>,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        linux-renesas-soc@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net-next 07/12] net: dsa: rzn1-a5psw: add statistics
 support
Message-ID: <20220415154420.128a0fca@fixe.home>
In-Reply-To: <Yll1IsIYKHC/n+sg@lunn.ch>
References: <20220414122250.158113-1-clement.leger@bootlin.com>
        <20220414122250.158113-8-clement.leger@bootlin.com>
        <YlirO7VrfyUH33rV@lunn.ch>
        <20220415140402.76822543@fixe.home>
        <Yll1IsIYKHC/n+sg@lunn.ch>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Le Fri, 15 Apr 2022 15:37:38 +0200,
Andrew Lunn <andrew@lunn.ch> a =C3=A9crit :

> > > > +static void a5psw_get_ethtool_stats(struct dsa_switch *ds, int por=
t,
> > > > +				    uint64_t *data)
> > > > +{
> > > > +	struct a5psw *a5psw =3D ds->priv;
> > > > +	u32 reg_lo, reg_hi;
> > > > +	unsigned int u;
> > > > +
> > > > +	for (u =3D 0; u < ARRAY_SIZE(a5psw_stats); u++) {
> > > > +		/* A5PSW_STATS_HIWORD is global and thus, access must be
> > > > +		 * exclusive
> > > > +		 */   =20
> > >=20
> > > Could you explain that a bit more. The RTNL lock will prevent two
> > > parallel calls to this function. =20
> >=20
> > Ok, I wasn't sure of the locking applicable here. =20
>=20
> In general, RTNL protects you for any user space management like
> operation on the driver. In this case, if you look in net/ethtool, you
> will find the IOCTL handler code takes RTNL before calling into the
> main IOCTL dispatcher. If you want to be paranoid/document the
> assumption, you can add an ASSERT_RTNL().

Ok, I'll look at the call stack in details to see what locking is
applied.

>=20
> The semantics for some of the other statistics Vladimir requested can
> be slightly different. One of them is in atomic context, because a
> spinlock is held. But i don't remember if RTNL is also held. This is
> less of an issue for your switch, since it uses MMIO, however many
> switches need to perform blocking IO over MDIO, SPI, IC2 etc to get
> stats, which you cannot do in atomic context. So they end up returning
> cached values.
>=20
> Look in the mailing list for past discussion for details.

Ok, thanks,

>=20
>     Andrew


--=20
Cl=C3=A9ment L=C3=A9ger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com