realtek: dsa: rtl83xx: flush scheduled work on removal

The workqueue items don't need to be processed directly when they are
scheduled. It can happen that they are simply processed at a much later
time. It is therefore necessary to ensure that all workqueue items of a
driver are no longer being processed before the driver (or structures of
this driver) are destroyed.

When skipping this step, the driver driver can cause a kernel Oops on
reboot.

Unfortunately, it is not recommended [1] to flush items out of the system
workqueue - simply because this can cause deadlocks. The driver itself must
have a private workqueue which is then flushed.

[1] https://lkml.kernel.org/r/49925af7-78a8-a3dd-bce6-cfc02e1a9236@I-love.SAKURA.ne.jp

Signed-off-by: Issam Hamdi <ih@simonwunderlich.de>
Signed-off-by: Harshal Gohel <hg@simonwunderlich.de>
Signed-off-by: Sharadanand Karanjkar <sk@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/19570
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>

diff --git a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c
index 7f96bdd82d1989ab4281fa94ffe02125f162b623..069faf7ed9100828d8b58daec0f33dd789e3da24 100644 (file)
--- a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c
+++ b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c
@@ -1330,7 +1330,7 @@ static int rtl83xx_netevent_event(struct notifier_block *this,
 
                pr_debug("%s: updating neighbour on port %d, mac %016llx\n",
                        __func__, port, net_work->mac);
-               schedule_work(&net_work->work);
+               queue_work(priv->wq, &net_work->work);
                if (err)
                        netdev_warn(dev, "failed to handle neigh update (err %d)\n", err);
                break;
@@ -1452,7 +1452,7 @@ static int rtl83xx_fib_event(struct notifier_block *this, unsigned long event, v
                break;
        }
 
-       schedule_work(&fib_work->work);
+       queue_work(priv->wq, &fib_work->work);
 
        return NOTIFY_DONE;
 }
@@ -1486,6 +1486,7 @@ static int __init rtl83xx_sw_probe(struct platform_device *pdev)
        priv->ds->ops = &rtl83xx_switch_ops;
        priv->ds->needs_standalone_vlan_filtering = true;
        priv->dev = dev;
+       dev_set_drvdata(dev, priv);
 
        err = devm_mutex_init(dev, &priv->reg_mutex);
        if (err)
@@ -1583,10 +1584,16 @@ static int __init rtl83xx_sw_probe(struct platform_device *pdev)
                return err;
        }
 
+       priv->wq = create_singlethread_workqueue("rtl83xx");
+       if (!priv->wq) {
+               dev_err(dev, "Error creating workqueue: %d\n", err);
+               return -ENOMEM;
+       }
+
        err = dsa_register_switch(priv->ds);
        if (err) {
                dev_err(dev, "Error registering switch: %d\n", err);
-               return err;
+               goto err_register_switch;
        }
 
        /* dsa_to_port returns dsa_port from the port list in
@@ -1694,13 +1701,37 @@ err_register_fib_nb:
 err_register_ne_nb:
        unregister_netdevice_notifier(&priv->nb);
 err_register_nb:
+       dsa_switch_shutdown(priv->ds);
+err_register_switch:
+       destroy_workqueue(priv->wq);
+
        return err;
 }
 
 static void rtl83xx_sw_remove(struct platform_device *pdev)
 {
+       struct rtl838x_switch_priv *priv = platform_get_drvdata(pdev);
+
+       if (!priv)
+               return;
+
        /* TODO: */
        pr_debug("Removing platform driver for rtl83xx-sw\n");
+
+       /* unregister notifiers which will create workqueue entries with
+        * references to the switch structures. Also stop self-arming delayed
+        * work items to avoid them still accessing the DSA structures
+        * when they are getting shut down.
+        */
+       unregister_fib_notifier(&init_net, &priv->fib_nb);
+       unregister_netevent_notifier(&priv->ne_nb);
+       cancel_delayed_work_sync(&priv->counters_work);
+
+       dsa_switch_shutdown(priv->ds);
+
+       destroy_workqueue(priv->wq);
+
+       dev_set_drvdata(&pdev->dev, NULL);
 }
 
 static const struct of_device_id rtl83xx_switch_of_ids[] = {

diff --git a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/dsa.c b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/dsa.c
index a14b502cf108249af20633afbc0a6b62ad3298f7..17a8b46f0c40cffe8429279e1dd122fb4cb319d1 100644 (file)
--- a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/dsa.c
+++ b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/dsa.c
@@ -1236,7 +1236,8 @@ static void rtldsa_poll_counters(struct work_struct *work)
                spin_unlock(&counters->lock);
        }
 
-       schedule_delayed_work(&priv->counters_work, RTLDSA_COUNTERS_POLL_INTERVAL);
+       queue_delayed_work(priv->wq, &priv->counters_work,
+                          RTLDSA_COUNTERS_POLL_INTERVAL);
 }
 
 static void rtldsa_init_counters(struct rtl838x_switch_priv *priv)
@@ -1254,7 +1255,8 @@ static void rtldsa_init_counters(struct rtl838x_switch_priv *priv)
        }
 
        INIT_DELAYED_WORK(&priv->counters_work, rtldsa_poll_counters);
-       schedule_delayed_work(&priv->counters_work, RTLDSA_COUNTERS_POLL_INTERVAL);
+       queue_delayed_work(priv->wq, &priv->counters_work,
+                          RTLDSA_COUNTERS_POLL_INTERVAL);
 }
 
 static void rtldsa_get_strings(struct dsa_switch *ds,

diff --git a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/rtl838x.h b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/rtl838x.h
index 896e9ab24b3d1e6e44b6ea99a12af8bf017c23c2..18405bdc02d9bdb35644f06946678608808d82e5 100644 (file)
--- a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/rtl838x.h
+++ b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/rtl838x.h
@@ -1143,6 +1143,7 @@ struct rtl838x_switch_priv {
        u32 lag_primary[MAX_LAGS];
        u32 is_lagmember[57];
        u64 lagmembers;
+       struct workqueue_struct *wq;
        struct notifier_block nb;  /* TODO: change to different name */
        struct notifier_block ne_nb;
        struct notifier_block fib_nb;