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 --- 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@makrotopia.org/ 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