[PATCH net v2] net: dsa: mxl862xx: disable the stats poll on teardown

[Daniel Golle]

mxl862xx_setup() arms the stats poll before mxl862xx_setup_mdio(), and
nothing stops it until dsa_register_switch() has returned an error to
mxl862xx_probe(). DSA frees the dsa_port list before it returns, so a
poll that fires once .setup or a later step of dsa_tree_setup() has
failed walks freed ports. On shutdown the user ports stay registered,
and the WORK_STOPPED flag test in mxl862xx_get_stats64() is not atomic
with the cancel in mxl862xx_shutdown(), so a re-arm that read the flag
before it was set queues the poll after cancel_delayed_work_sync() has
returned.

Arm the poll once .setup has succeeded and stop it from a .teardown op,
which DSA calls on unregister and after a failed registration, in both
cases before it frees the ports. Use disable_delayed_work_sync() there
and in shutdown(): it drains a running poll as the cancel did and turns
every later attempt to queue the work into a no-op, so the re-arm
cannot bring the poll back. remove() and the probe error path only set
WORK_STOPPED, which crc_err_work tests before it walks the ports.

Fixes: a21d33a5265f ("net: dsa: mxl862xx: implement .get_stats64")
Assisted-by: LLM
Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
---
v2:
 - arm the poll after .setup has succeeded and stop it from .teardown,
   before DSA frees the ports when registration fails (found by Sashiko
   AI review; Paolo Abeni suggested addressing it in the same patch)
 - the changelog names the get_stats64() re-arm as the one enqueue that
   can land after the cancel, as cancel_delayed_work_sync() keeps the
   work disabled while the poll's own re-arm runs (found by Sashiko AI
   review)
 - the comment above the get_stats64() re-arm keeps only its purpose
   (found by Sashiko AI review)

v1: https://lore.kernel.org/all/8b861014c836377afab0fdfb66a83fa017e5cd84.1788779062.git.daniel@xxxxxxxxxxxxxx/

 drivers/net/dsa/mxl862xx/mxl862xx.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.c b/drivers/net/dsa/mxl862xx/mxl862xx.c
index cfa7e3e269a2..e05ad52cd297 100644
--- a/drivers/net/dsa/mxl862xx/mxl862xx.c
+++ b/drivers/net/dsa/mxl862xx/mxl862xx.c
@@ -685,10 +685,22 @@ static int mxl862xx_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = mxl862xx_setup_mdio(ds);
+	if (ret)
+		return ret;
+
 	schedule_delayed_work(&priv->stats_work,
 			      MXL862XX_STATS_POLL_INTERVAL);
 
-	return mxl862xx_setup_mdio(ds);
+	return 0;
+}
+
+static void mxl862xx_teardown(struct dsa_switch *ds)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+
+	set_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags);
+	disable_delayed_work_sync(&priv->stats_work);
 }
 
 static int mxl862xx_port_state(struct dsa_switch *ds, int port, bool enable)
@@ -2047,9 +2059,7 @@ static void mxl862xx_get_stats64(struct dsa_switch *ds, int port,
 
 	spin_unlock_bh(&priv->ports[port].stats_lock);
 
-	/* Trigger a fresh poll so the next read sees up-to-date counters.
-	 * No-op if the work is already pending, running, or teardown started.
-	 */
+	/* Trigger a fresh poll so the next read sees up-to-date counters. */
 	if (!test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags))
 		schedule_delayed_work(&priv->stats_work, 0);
 }
@@ -2057,6 +2067,7 @@ static void mxl862xx_get_stats64(struct dsa_switch *ds, int port,
 static const struct dsa_switch_ops mxl862xx_switch_ops = {
 	.get_tag_protocol = mxl862xx_get_tag_protocol,
 	.setup = mxl862xx_setup,
+	.teardown = mxl862xx_teardown,
 	.port_setup = mxl862xx_port_setup,
 	.port_teardown = mxl862xx_port_teardown,
 	.phylink_get_caps = mxl862xx_phylink_get_caps,
@@ -2131,7 +2142,6 @@ static int mxl862xx_probe(struct mdio_device *mdiodev)
 	err = dsa_register_switch(ds);
 	if (err) {
 		set_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags);
-		cancel_delayed_work_sync(&priv->stats_work);
 		mxl862xx_host_shutdown(priv);
 		for (i = 0; i < MXL862XX_MAX_PORTS; i++)
 			cancel_work_sync(&priv->ports[i].host_flood_work);
@@ -2152,7 +2162,6 @@ static void mxl862xx_remove(struct mdio_device *mdiodev)
 	priv = ds->priv;
 
 	set_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags);
-	cancel_delayed_work_sync(&priv->stats_work);
 
 	dsa_unregister_switch(ds);
 
@@ -2181,7 +2190,7 @@ static void mxl862xx_shutdown(struct mdio_device *mdiodev)
 	dsa_switch_shutdown(ds);
 
 	set_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags);
-	cancel_delayed_work_sync(&priv->stats_work);
+	disable_delayed_work_sync(&priv->stats_work);
 
 	mxl862xx_host_shutdown(priv);
 

base-commit: 7addb4e5ef1702704914b47bca3f706ef96c1589
-- 
2.55.0