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:37:38 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= <clement.leger@bootlin.com>
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>,
        =?iso-8859-1?Q?Miqu=E8l?= 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: <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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220415140402.76822543@fixe.home>
Precedence: bulk

> > > +static void a5psw_get_ethtool_stats(struct dsa_switch *ds, int port,
> > > +				    uint64_t *data)
> > > +{
> > > +	struct a5psw *a5psw = ds->priv;
> > > +	u32 reg_lo, reg_hi;
> > > +	unsigned int u;
> > > +
> > > +	for (u = 0; u < ARRAY_SIZE(a5psw_stats); u++) {
> > > +		/* A5PSW_STATS_HIWORD is global and thus, access must be
> > > +		 * exclusive
> > > +		 */  
> > 
> > Could you explain that a bit more. The RTNL lock will prevent two
> > parallel calls to this function.
> 
> Ok, I wasn't sure of the locking applicable here.

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().

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.

Look in the mailing list for past discussion for details.

    Andrew