mirror of
https://git.openwrt.org/openwrt/openwrt.git
synced 2026-09-09 00:06:08 +08:00
generic: pse-pd: rework the phydev->psec notifier patch to fix rtnl deadlock
The notifier patch attached phydev->psec under rtnl_lock() inside phy_device_register(). Drivers that register their MDIO bus from ndo_init() (like the lantiq etop) already hold rtnl at that point, so the attach tried to take rtnl a second time and deadlocked on probe. Aleksander hit this on lantiq arx100. Rework 896-03 to use a dedicated mutex instead of rtnl for the psec attach, the notifier walks and the ethtool PSE paths, so there is no rtnl recursion any more. The mutex lives in pse_core.c rather than in phylib: net/ethtool is always built into vmlinux while PHYLIB is tristate, so net/ethtool/pse-pd.c must not call a phylib export or CONFIG_PHYLIB=m fails to link. PSE_CONTROLLER is bool, so pse_core is either in vmlinux or absent and every config can reach pse_phy_lock()/pse_phy_unlock(); !PSE_CONTROLLER gets no-op stubs in pse.h. phy_device_register_locked() is gone with it. It only existed because the attach took rtnl, and was identical to phy_device_register() apart from an ASSERT_RTNL(), so sfp.c calls phy_device_register() again and include/linux/phy.h stays untouched. Also refresh 897-01 and 897-02, whose hunks shift. Tested-by: Aleksander Jan Bajkowski <olek2@wp.pl> Signed-off-by: Carlo Szelinsky <github@szelinsky.de> Link: https://github.com/openwrt/openwrt/pull/24945 Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
This commit is contained in:
+253
-153
@@ -1,51 +1,56 @@
|
|||||||
|
From 6b075effc279665115941d8bd5c2116d9b454d42 Mon Sep 17 00:00:00 2001
|
||||||
From: Corey Leavitt <corey@leavitt.info>
|
From: Corey Leavitt <corey@leavitt.info>
|
||||||
|
Date: Thu, 23 Apr 2026 01:42:17 -0600
|
||||||
|
Subject: [PATCH] net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio hook
|
||||||
|
|
||||||
Transfer ownership of phydev->psec from fwnode_mdio to the phy
|
Transfer ownership of phydev->psec from fwnode_mdio to the phy
|
||||||
subsystem itself. The phy subsystem now subscribes to the pse-pd
|
subsystem itself. The phy subsystem subscribes to the pse-pd notifier
|
||||||
notifier chain and manages psec attach/detach in response to PSE
|
chain and manages psec attach/detach in response to PSE controller
|
||||||
controller lifecycle events, while fwnode_mdio loses its PSE awareness
|
lifecycle events, while fwnode_mdio loses its PSE awareness entirely.
|
||||||
entirely.
|
|
||||||
|
|
||||||
phydev->psec is attached after device_add() has made the phy visible
|
phydev->psec is attached after device_add() has made the phy visible on
|
||||||
on mdio_bus_type, under a narrow rtnl_lock() that covers only
|
mdio_bus_type. Ordering the attach after registration closes the race
|
||||||
phy_try_attach_pse(). Ordering the attach after registration closes
|
that would otherwise leave a phy unattached: a PSE_REGISTERED event
|
||||||
the race that would otherwise leave a phy unattached: a PSE_REGISTERED
|
firing during registration walks mdio_bus_type and either finds the phy
|
||||||
event firing during registration walks mdio_bus_type and either finds
|
already added (and attaches it) or runs before device_add(), in which
|
||||||
the phy already added (and attaches it) or runs before device_add(),
|
case the post-add attach resolves it. The phydev->psec check in
|
||||||
in which case the post-add attach resolves it. The phydev->psec check
|
phy_try_attach_pse() makes the two paths idempotent.
|
||||||
in phy_try_attach_pse() makes the two paths idempotent. Holding rtnl
|
|
||||||
across of_pse_control_get() is safe because pse_list_mutex is never
|
|
||||||
taken in the opposite order.
|
|
||||||
|
|
||||||
device_add() is deliberately left outside rtnl. Binding a phy that
|
A dedicated pse_phy_mutex in pse_core.c, taken through pse_phy_lock(),
|
||||||
|
serialises the attach against the PSE controller notifier walk and
|
||||||
|
against the ethtool PSE paths that dereference phydev->psec. It is used
|
||||||
|
instead of rtnl on purpose: an MDIO bus registered from ndo_init()
|
||||||
|
(e.g. lantiq_etop) calls phy_device_register() with rtnl already held,
|
||||||
|
so taking rtnl for the attach would deadlock. The ethtool PSE reads in
|
||||||
|
net/ethtool/pse-pd.c take the same lock so the PSE_UNREGISTERED detach
|
||||||
|
cannot free phydev->psec underneath them. The lock order is
|
||||||
|
rtnl -> pse_phy_mutex -> pse_list_mutex -> pcdev->lock, and the notifier
|
||||||
|
walks enter at pse_phy_lock() and never take rtnl.
|
||||||
|
|
||||||
|
The lock lives in pse_core rather than in phylib because PSE_CONTROLLER
|
||||||
|
is bool while phylib is tristate: net/ethtool is always built into
|
||||||
|
vmlinux, so with CONFIG_PHYLIB=m it cannot call a phylib export.
|
||||||
|
pse_core is either in vmlinux or absent, so every config can reach it,
|
||||||
|
and !PSE_CONTROLLER gets no-op stubs in pse.h.
|
||||||
|
|
||||||
|
device_add() is deliberately left outside the lock. Binding a phy that
|
||||||
itself provides an SFP cage reaches sfp_bus_add_upstream() through
|
itself provides an SFP cage reaches sfp_bus_add_upstream() through
|
||||||
phy_probe() -> phy_setup_ports() -> phy_sfp_probe(), and
|
phy_probe() -> phy_setup_ports() -> phy_sfp_probe(), and
|
||||||
sfp_bus_add_upstream() takes rtnl_lock(); holding rtnl across
|
sfp_bus_add_upstream() takes rtnl_lock(); holding pse_phy_mutex across
|
||||||
device_add() would deadlock such phys (reported on RTL8214FC).
|
device_add() would invert that lock order (reported on RTL8214FC).
|
||||||
|
|
||||||
phy_device_register() is split into the public form, which takes the
|
- On PSE_REGISTERED: a bus walk retries the attach for every
|
||||||
narrow rtnl_lock() around the attach, and a phy_device_register_locked()
|
registered phy whose psec is still NULL. This is the "phy was
|
||||||
form for callers that already hold rtnl (the SFP module state machine
|
enumerated before the PSE controller loaded" case, the root cause of
|
||||||
via __sfp_sm_event). This pair mirrors the register_netdevice() /
|
the boot-time probe-retry storm on systems with a modular PSE
|
||||||
register_netdev() split convention already established in the core
|
controller driver.
|
||||||
networking stack. The _locked form runs device_add() under the
|
|
||||||
caller's rtnl, which is safe because a phy resident on an SFP module
|
|
||||||
does not itself provide a downstream cage, so phy_sfp_probe() is a
|
|
||||||
no-op there.
|
|
||||||
|
|
||||||
- On PSE_REGISTERED: an rtnl-guarded bus walk retries the attach for
|
- On PSE_UNREGISTERED: a bus walk releases every phydev->psec that
|
||||||
every registered phy whose psec is still NULL. This is the "phy
|
targets the departing controller before pse_release_pis() frees
|
||||||
was enumerated before the PSE controller loaded" case, the root
|
pcdev->pi. Without this, a phy still holding a pse_control reference
|
||||||
cause of the boot-time probe-retry storm on systems with a modular
|
would cause a use-after-free in __pse_control_release()'s
|
||||||
PSE controller driver.
|
pcdev->pi[psec->id] access, and the PSE driver module could not
|
||||||
|
finish unloading while any phy still held a reference.
|
||||||
- On PSE_UNREGISTERED: an rtnl-guarded bus walk releases every
|
|
||||||
phydev->psec that targets the departing controller before
|
|
||||||
pse_release_pis() frees pcdev->pi. Without this, a phy still
|
|
||||||
holding a pse_control reference would cause a use-after-free in
|
|
||||||
__pse_control_release()'s pcdev->pi[psec->id] access, and the PSE
|
|
||||||
driver module could not finish unloading while any phy still held a
|
|
||||||
reference.
|
|
||||||
|
|
||||||
A bad `pses` binding -- an error from of_pse_control_get() other than
|
A bad `pses` binding -- an error from of_pse_control_get() other than
|
||||||
-ENOENT (no phandle) or -EPROBE_DEFER (controller not yet registered)
|
-ENOENT (no phandle) or -EPROBE_DEFER (controller not yet registered)
|
||||||
@@ -66,18 +71,20 @@ now PSE-agnostic.
|
|||||||
|
|
||||||
Reported-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
Reported-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
||||||
Closes: https://lore.kernel.org/netdev/e00048dd-1ed3-40c3-9912-59bccf015ad5@gmail.com/
|
Closes: https://lore.kernel.org/netdev/e00048dd-1ed3-40c3-9912-59bccf015ad5@gmail.com/
|
||||||
|
Reported-by: Aleksander Jan Bajkowski <olek2@wp.pl>
|
||||||
|
Closes: https://lore.kernel.org/netdev/bac5e6e9-7358-4ccb-87fc-9c40baa33682@wp.pl/
|
||||||
Signed-off-by: Corey Leavitt <corey@leavitt.info>
|
Signed-off-by: Corey Leavitt <corey@leavitt.info>
|
||||||
Co-developed-by: Carlo Szelinsky <github@szelinsky.de>
|
Co-developed-by: Carlo Szelinsky <github@szelinsky.de>
|
||||||
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
||||||
Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
||||||
|
Tested-by: Aleksander Jan Bajkowski <olek2@wp.pl>
|
||||||
---
|
---
|
||||||
drivers/net/mdio/fwnode_mdio.c | 34 -------
|
drivers/net/mdio/fwnode_mdio.c | 34 -----------
|
||||||
drivers/net/phy/phy_device.c | 168 +++++++++++++++++++++++++++++++--
|
drivers/net/phy/phy_device.c | 126 +++++++++++++++++++++++++++++++++++++++-
|
||||||
drivers/net/phy/sfp.c | 2 +-
|
drivers/net/pse-pd/pse_core.c | 60 +++++++++++++++++++
|
||||||
drivers/net/pse-pd/pse_core.c | 14 +++
|
include/linux/pse-pd/pse.h | 32 ++++++++++
|
||||||
include/linux/phy.h | 2 +
|
net/ethtool/pse-pd.c | 22 +++++--
|
||||||
include/linux/pse-pd/pse.h | 9 ++
|
5 files changed, 231 insertions(+), 43 deletions(-)
|
||||||
6 files changed, 186 insertions(+), 43 deletions(-)
|
|
||||||
|
|
||||||
--- a/drivers/net/mdio/fwnode_mdio.c
|
--- a/drivers/net/mdio/fwnode_mdio.c
|
||||||
+++ b/drivers/net/mdio/fwnode_mdio.c
|
+++ b/drivers/net/mdio/fwnode_mdio.c
|
||||||
@@ -171,26 +178,22 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void phy_mdio_device_remove(struct mdio_device *mdiodev)
|
static void phy_mdio_device_remove(struct mdio_device *mdiodev)
|
||||||
@@ -1138,11 +1149,103 @@ struct phy_device *get_phy_device(struct
|
@@ -1138,9 +1149,108 @@ struct phy_device *get_phy_device(struct
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL(get_phy_device);
|
EXPORT_SYMBOL(get_phy_device);
|
||||||
|
|
||||||
-/**
|
|
||||||
- * phy_device_register - Register the phy device on the MDIO bus
|
|
||||||
- * @phydev: phy_device structure to be added to the MDIO bus
|
|
||||||
+/* Best-effort attach of phydev->psec from a DT `pses = <&...>` phandle.
|
+/* Best-effort attach of phydev->psec from a DT `pses = <&...>` phandle.
|
||||||
+ * Caller must hold rtnl. A missing phandle (-ENOENT) or a not-yet-registered
|
+ * Caller must hold pse_phy_lock(). A missing phandle (-ENOENT) or a
|
||||||
+ * controller (-EPROBE_DEFER) is silent; the notifier retries the latter at
|
+ * not-yet-registered controller (-EPROBE_DEFER) is silent; the notifier
|
||||||
+ * PSE_REGISTERED time. Any other error means a broken binding and is warned
|
+ * retries the latter at PSE_REGISTERED time. Any other error means a broken
|
||||||
+ * about, but left non-fatal so the phy still registers.
|
+ * binding and is warned about, but left non-fatal so the phy still registers.
|
||||||
*/
|
+ */
|
||||||
-int phy_device_register(struct phy_device *phydev)
|
|
||||||
+static void phy_try_attach_pse(struct phy_device *phydev)
|
+static void phy_try_attach_pse(struct phy_device *phydev)
|
||||||
+{
|
+{
|
||||||
+ struct pse_control *psec;
|
+ struct pse_control *psec;
|
||||||
+ struct device_node *np;
|
+ struct device_node *np;
|
||||||
+
|
+
|
||||||
+ ASSERT_RTNL();
|
+ pse_phy_lock_assert_held();
|
||||||
+
|
+
|
||||||
+ np = phydev->mdio.dev.of_node;
|
+ np = phydev->mdio.dev.of_node;
|
||||||
+ if (!np)
|
+ if (!np)
|
||||||
@@ -212,7 +215,7 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
+
|
+
|
||||||
+static int phy_pse_attach_one(struct device *dev, void *data __maybe_unused)
|
+static int phy_pse_attach_one(struct device *dev, void *data __maybe_unused)
|
||||||
+{
|
+{
|
||||||
+ ASSERT_RTNL();
|
+ pse_phy_lock_assert_held();
|
||||||
+
|
+
|
||||||
+ if (dev->type != &mdio_bus_phy_type)
|
+ if (dev->type != &mdio_bus_phy_type)
|
||||||
+ return 0;
|
+ return 0;
|
||||||
@@ -227,7 +230,7 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
+ struct phy_device *phydev;
|
+ struct phy_device *phydev;
|
||||||
+ struct pse_control *psec;
|
+ struct pse_control *psec;
|
||||||
+
|
+
|
||||||
+ ASSERT_RTNL();
|
+ pse_phy_lock_assert_held();
|
||||||
+
|
+
|
||||||
+ if (dev->type != &mdio_bus_phy_type)
|
+ if (dev->type != &mdio_bus_phy_type)
|
||||||
+ return 0;
|
+ return 0;
|
||||||
@@ -247,16 +250,16 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
+{
|
+{
|
||||||
+ switch (event) {
|
+ switch (event) {
|
||||||
+ case PSE_REGISTERED:
|
+ case PSE_REGISTERED:
|
||||||
+ rtnl_lock();
|
+ pse_phy_lock();
|
||||||
+ bus_for_each_dev(&mdio_bus_type, NULL, NULL,
|
+ bus_for_each_dev(&mdio_bus_type, NULL, NULL,
|
||||||
+ phy_pse_attach_one);
|
+ phy_pse_attach_one);
|
||||||
+ rtnl_unlock();
|
+ pse_phy_unlock();
|
||||||
+ return NOTIFY_OK;
|
+ return NOTIFY_OK;
|
||||||
+ case PSE_UNREGISTERED:
|
+ case PSE_UNREGISTERED:
|
||||||
+ rtnl_lock();
|
+ pse_phy_lock();
|
||||||
+ bus_for_each_dev(&mdio_bus_type, NULL, data,
|
+ bus_for_each_dev(&mdio_bus_type, NULL, data,
|
||||||
+ phy_pse_detach_one);
|
+ phy_pse_detach_one);
|
||||||
+ rtnl_unlock();
|
+ pse_phy_unlock();
|
||||||
+ return NOTIFY_OK;
|
+ return NOTIFY_OK;
|
||||||
+ default:
|
+ default:
|
||||||
+ return NOTIFY_DONE;
|
+ return NOTIFY_DONE;
|
||||||
@@ -267,19 +270,33 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
+ .notifier_call = phy_pse_notifier_event,
|
+ .notifier_call = phy_pse_notifier_event,
|
||||||
+};
|
+};
|
||||||
+
|
+
|
||||||
+/* Core registration: add the phy to the MDIO bus. Does not touch rtnl or
|
/**
|
||||||
+ * PSE. phydev->psec is attached by the callers below, after device_add()
|
* phy_device_register - Register the phy device on the MDIO bus
|
||||||
+ * has made the phy visible on mdio_bus_type, so that a concurrent PSE
|
* @phydev: phy_device structure to be added to the MDIO bus
|
||||||
+ * notifier walk and the attach can never leave the phy unattached. Keeping
|
+ *
|
||||||
+ * device_add() out of rtnl also avoids deadlocking when binding a phy that
|
+ * phydev->psec is attached after device_add() has made the phy visible on
|
||||||
+ * itself provides an SFP cage (phy_probe() -> phy_sfp_probe() ->
|
+ * mdio_bus_type, so that a concurrent PSE notifier walk and the attach can
|
||||||
+ * sfp_bus_add_upstream() takes rtnl).
|
+ * never leave the phy unattached. Neither step takes rtnl: keeping
|
||||||
+ */
|
+ * device_add() out of rtnl avoids deadlocking when binding a phy that itself
|
||||||
+static int __phy_device_register(struct phy_device *phydev)
|
+ * provides an SFP cage (phy_probe() -> phy_sfp_probe() ->
|
||||||
|
+ * sfp_bus_add_upstream() takes rtnl), and pse_phy_lock() rather than rtnl
|
||||||
|
+ * guards the attach so a bus registered from ndo_init (which already holds
|
||||||
|
+ * rtnl) does not recurse on it.
|
||||||
|
+ *
|
||||||
|
+ * Return: 0 on success, negative error code on failure.
|
||||||
|
*/
|
||||||
|
int phy_device_register(struct phy_device *phydev)
|
||||||
{
|
{
|
||||||
int err;
|
@@ -1166,12 +1276,15 @@ int phy_device_register(struct phy_devic
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
|
|
||||||
|
+ pse_phy_lock();
|
||||||
|
+ phy_try_attach_pse(phydev);
|
||||||
|
+ pse_phy_unlock();
|
||||||
|
+
|
||||||
|
return 0;
|
||||||
|
|
||||||
@@ -1171,10 +1274,54 @@ int phy_device_register(struct phy_devic
|
|
||||||
out:
|
out:
|
||||||
/* Assert the reset signal */
|
/* Assert the reset signal */
|
||||||
phy_device_reset(phydev, 1);
|
phy_device_reset(phydev, 1);
|
||||||
@@ -287,55 +304,7 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
mdiobus_unregister_device(&phydev->mdio);
|
mdiobus_unregister_device(&phydev->mdio);
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
+
|
@@ -1188,8 +1301,6 @@ EXPORT_SYMBOL(phy_device_register);
|
||||||
+/**
|
|
||||||
+ * phy_device_register_locked - Register the phy device on the MDIO bus
|
|
||||||
+ * @phydev: phy_device structure to be added to the MDIO bus
|
|
||||||
+ *
|
|
||||||
+ * Same as phy_device_register() but caller must already hold rtnl_lock().
|
|
||||||
+ *
|
|
||||||
+ * Return: 0 on success, negative error code on failure.
|
|
||||||
+ */
|
|
||||||
+int phy_device_register_locked(struct phy_device *phydev)
|
|
||||||
+{
|
|
||||||
+ int err;
|
|
||||||
+
|
|
||||||
+ ASSERT_RTNL();
|
|
||||||
+
|
|
||||||
+ err = __phy_device_register(phydev);
|
|
||||||
+ if (err)
|
|
||||||
+ return err;
|
|
||||||
+
|
|
||||||
+ phy_try_attach_pse(phydev);
|
|
||||||
+
|
|
||||||
+ return 0;
|
|
||||||
+}
|
|
||||||
+EXPORT_SYMBOL(phy_device_register_locked);
|
|
||||||
+
|
|
||||||
+/**
|
|
||||||
+ * phy_device_register - Register the phy device on the MDIO bus
|
|
||||||
+ * @phydev: phy_device structure to be added to the MDIO bus
|
|
||||||
+ *
|
|
||||||
+ * Return: 0 on success, negative error code on failure.
|
|
||||||
+ */
|
|
||||||
+int phy_device_register(struct phy_device *phydev)
|
|
||||||
+{
|
|
||||||
+ int err;
|
|
||||||
+
|
|
||||||
+ err = __phy_device_register(phydev);
|
|
||||||
+ if (err)
|
|
||||||
+ return err;
|
|
||||||
+
|
|
||||||
+ rtnl_lock();
|
|
||||||
+ phy_try_attach_pse(phydev);
|
|
||||||
+ rtnl_unlock();
|
|
||||||
+
|
|
||||||
+ return 0;
|
|
||||||
+}
|
|
||||||
EXPORT_SYMBOL(phy_device_register);
|
|
||||||
|
|
||||||
/**
|
|
||||||
@@ -1188,8 +1335,6 @@ EXPORT_SYMBOL(phy_device_register);
|
|
||||||
void phy_device_remove(struct phy_device *phydev)
|
void phy_device_remove(struct phy_device *phydev)
|
||||||
{
|
{
|
||||||
unregister_mii_timestamper(phydev->mii_ts);
|
unregister_mii_timestamper(phydev->mii_ts);
|
||||||
@@ -344,7 +313,7 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
device_del(&phydev->mdio.dev);
|
device_del(&phydev->mdio.dev);
|
||||||
|
|
||||||
/* Assert the reset signal */
|
/* Assert the reset signal */
|
||||||
@@ -3726,8 +3871,14 @@ static int __init phy_init(void)
|
@@ -3726,8 +3837,14 @@ static int __init phy_init(void)
|
||||||
if (rc)
|
if (rc)
|
||||||
goto err_c45;
|
goto err_c45;
|
||||||
|
|
||||||
@@ -359,7 +328,7 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
err_c45:
|
err_c45:
|
||||||
phy_driver_unregister(&genphy_c45_driver);
|
phy_driver_unregister(&genphy_c45_driver);
|
||||||
err_ethtool_phy_ops:
|
err_ethtool_phy_ops:
|
||||||
@@ -3741,6 +3892,7 @@ err_ethtool_phy_ops:
|
@@ -3741,6 +3858,7 @@ err_ethtool_phy_ops:
|
||||||
|
|
||||||
static void __exit phy_exit(void)
|
static void __exit phy_exit(void)
|
||||||
{
|
{
|
||||||
@@ -367,20 +336,65 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
phy_driver_unregister(&genphy_c45_driver);
|
phy_driver_unregister(&genphy_c45_driver);
|
||||||
phy_driver_unregister(&genphy_driver);
|
phy_driver_unregister(&genphy_driver);
|
||||||
rtnl_lock();
|
rtnl_lock();
|
||||||
--- a/drivers/net/phy/sfp.c
|
|
||||||
+++ b/drivers/net/phy/sfp.c
|
|
||||||
@@ -1995,7 +1995,7 @@ static int sfp_sm_probe_phy(struct sfp *
|
|
||||||
/* Mark this PHY as being on a SFP module */
|
|
||||||
phy->is_on_sfp_module = true;
|
|
||||||
|
|
||||||
- err = phy_device_register(phy);
|
|
||||||
+ err = phy_device_register_locked(phy);
|
|
||||||
if (err) {
|
|
||||||
phy_device_free(phy);
|
|
||||||
dev_err(sfp->dev, "phy_device_register failed: %pe\n",
|
|
||||||
--- a/drivers/net/pse-pd/pse_core.c
|
--- a/drivers/net/pse-pd/pse_core.c
|
||||||
+++ b/drivers/net/pse-pd/pse_core.c
|
+++ b/drivers/net/pse-pd/pse_core.c
|
||||||
@@ -2028,3 +2028,17 @@ bool pse_has_c33(struct pse_control *pse
|
@@ -24,9 +24,55 @@ static LIST_HEAD(pse_controller_list);
|
||||||
|
static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
|
||||||
|
static DEFINE_MUTEX(pse_pw_d_mutex);
|
||||||
|
|
||||||
|
+/* Serialises phydev->psec against the PSE controller lifecycle notifier and
|
||||||
|
+ * the ethtool PSE paths, in place of rtnl. The attach must not take rtnl: an
|
||||||
|
+ * MDIO bus registered from ndo_init (e.g. lantiq_etop) calls
|
||||||
|
+ * phy_device_register() with rtnl already held, so taking rtnl for the attach
|
||||||
|
+ * would deadlock. It lives here rather than in phylib because PSE_CONTROLLER
|
||||||
|
+ * is bool, so pse_core is either in vmlinux or absent, and net/ethtool can
|
||||||
|
+ * call these directly; phylib is tristate and must not be linked against
|
||||||
|
+ * from built-in code. Lock order: rtnl -> pse_phy_mutex -> pse_list_mutex ->
|
||||||
|
+ * pcdev->lock.
|
||||||
|
+ */
|
||||||
|
+static DEFINE_MUTEX(pse_phy_mutex);
|
||||||
|
+
|
||||||
|
static BLOCKING_NOTIFIER_HEAD(pse_controller_notifier);
|
||||||
|
|
||||||
|
/**
|
||||||
|
+ * pse_phy_lock - hold phydev->psec stable against PSE controller teardown
|
||||||
|
+ *
|
||||||
|
+ * The PSE_UNREGISTERED notifier clears phydev->psec and drops the last
|
||||||
|
+ * reference on the pse_control before the controller frees its state. Callers
|
||||||
|
+ * that attach, detach or dereference phydev->psec must hold this lock across
|
||||||
|
+ * the whole access so the detach cannot run underneath them.
|
||||||
|
+ */
|
||||||
|
+void pse_phy_lock(void)
|
||||||
|
+{
|
||||||
|
+ mutex_lock(&pse_phy_mutex);
|
||||||
|
+}
|
||||||
|
+EXPORT_SYMBOL_GPL(pse_phy_lock);
|
||||||
|
+
|
||||||
|
+/**
|
||||||
|
+ * pse_phy_unlock - release the lock taken by pse_phy_lock()
|
||||||
|
+ */
|
||||||
|
+void pse_phy_unlock(void)
|
||||||
|
+{
|
||||||
|
+ mutex_unlock(&pse_phy_mutex);
|
||||||
|
+}
|
||||||
|
+EXPORT_SYMBOL_GPL(pse_phy_unlock);
|
||||||
|
+
|
||||||
|
+#ifdef CONFIG_LOCKDEP
|
||||||
|
+/**
|
||||||
|
+ * pse_phy_lock_assert_held - assert that pse_phy_lock() is held
|
||||||
|
+ */
|
||||||
|
+void pse_phy_lock_assert_held(void)
|
||||||
|
+{
|
||||||
|
+ lockdep_assert_held(&pse_phy_mutex);
|
||||||
|
+}
|
||||||
|
+EXPORT_SYMBOL_GPL(pse_phy_lock_assert_held);
|
||||||
|
+#endif
|
||||||
|
+
|
||||||
|
+/**
|
||||||
|
* pse_register_notifier - register a callback for PSE controller events
|
||||||
|
* @nb: notifier block to register
|
||||||
|
*
|
||||||
|
@@ -2028,3 +2074,17 @@ bool pse_has_c33(struct pse_control *pse
|
||||||
return psec->pcdev->types & ETHTOOL_PSE_C33;
|
return psec->pcdev->types & ETHTOOL_PSE_C33;
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(pse_has_c33);
|
EXPORT_SYMBOL_GPL(pse_has_c33);
|
||||||
@@ -398,20 +412,9 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
+ return psec->pcdev == pcdev;
|
+ return psec->pcdev == pcdev;
|
||||||
+}
|
+}
|
||||||
+EXPORT_SYMBOL_GPL(pse_control_matches_pcdev);
|
+EXPORT_SYMBOL_GPL(pse_control_matches_pcdev);
|
||||||
--- a/include/linux/phy.h
|
|
||||||
+++ b/include/linux/phy.h
|
|
||||||
@@ -1860,6 +1860,8 @@ struct phy_device *fwnode_phy_find_devic
|
|
||||||
struct fwnode_handle *fwnode_get_phy_node(const struct fwnode_handle *fwnode);
|
|
||||||
struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45);
|
|
||||||
int phy_device_register(struct phy_device *phy);
|
|
||||||
+/* Caller must hold rtnl_lock(); see phy_device_register() for the public form. */
|
|
||||||
+int phy_device_register_locked(struct phy_device *phy);
|
|
||||||
void phy_device_free(struct phy_device *phydev);
|
|
||||||
void phy_device_remove(struct phy_device *phydev);
|
|
||||||
int phy_get_c45_ids(struct phy_device *phydev);
|
|
||||||
--- a/include/linux/pse-pd/pse.h
|
--- a/include/linux/pse-pd/pse.h
|
||||||
+++ b/include/linux/pse-pd/pse.h
|
+++ b/include/linux/pse-pd/pse.h
|
||||||
@@ -385,6 +385,9 @@ int pse_ethtool_set_prio(struct pse_cont
|
@@ -385,9 +385,23 @@ int pse_ethtool_set_prio(struct pse_cont
|
||||||
bool pse_has_podl(struct pse_control *psec);
|
bool pse_has_podl(struct pse_control *psec);
|
||||||
bool pse_has_c33(struct pse_control *psec);
|
bool pse_has_c33(struct pse_control *psec);
|
||||||
|
|
||||||
@@ -421,16 +424,113 @@ Tested-by: Jonas Jelonek <jelonek.jonas@gmail.com>
|
|||||||
int pse_register_notifier(struct notifier_block *nb);
|
int pse_register_notifier(struct notifier_block *nb);
|
||||||
int pse_unregister_notifier(struct notifier_block *nb);
|
int pse_unregister_notifier(struct notifier_block *nb);
|
||||||
|
|
||||||
@@ -437,6 +440,12 @@ static inline bool pse_has_c33(struct ps
|
+void pse_phy_lock(void);
|
||||||
{
|
+void pse_phy_unlock(void);
|
||||||
|
+
|
||||||
|
+#ifdef CONFIG_LOCKDEP
|
||||||
|
+void pse_phy_lock_assert_held(void);
|
||||||
|
+#else
|
||||||
|
+static inline void pse_phy_lock_assert_held(void)
|
||||||
|
+{
|
||||||
|
+}
|
||||||
|
+#endif
|
||||||
|
+
|
||||||
|
#else
|
||||||
|
|
||||||
|
static inline struct pse_control *of_pse_control_get(struct device_node *node,
|
||||||
|
@@ -438,6 +452,12 @@ static inline bool pse_has_c33(struct ps
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
+
|
|
||||||
+static inline bool pse_control_matches_pcdev(struct pse_control *psec,
|
+static inline bool pse_control_matches_pcdev(struct pse_control *psec,
|
||||||
+ struct pse_controller_dev *pcdev)
|
+ struct pse_controller_dev *pcdev)
|
||||||
+{
|
+{
|
||||||
+ return false;
|
+ return false;
|
||||||
+}
|
+}
|
||||||
|
+
|
||||||
static inline int pse_register_notifier(struct notifier_block *nb)
|
static inline int pse_register_notifier(struct notifier_block *nb)
|
||||||
{
|
{
|
||||||
|
return 0;
|
||||||
|
@@ -448,6 +468,18 @@ static inline int pse_unregister_notifie
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
+static inline void pse_phy_lock(void)
|
||||||
|
+{
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+static inline void pse_phy_unlock(void)
|
||||||
|
+{
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+static inline void pse_phy_lock_assert_held(void)
|
||||||
|
+{
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
#endif
|
||||||
|
|
||||||
|
#endif
|
||||||
|
--- a/net/ethtool/pse-pd.c
|
||||||
|
+++ b/net/ethtool/pse-pd.c
|
||||||
|
@@ -70,7 +70,12 @@ static int pse_prepare_data(const struct
|
||||||
|
if (ret < 0)
|
||||||
|
return ret;
|
||||||
|
|
||||||
|
+ /* Hold phydev->psec stable against a PSE controller unregister that
|
||||||
|
+ * would detach and free it while it is being dereferenced.
|
||||||
|
+ */
|
||||||
|
+ pse_phy_lock();
|
||||||
|
ret = pse_get_pse_attributes(phydev, info->extack, data);
|
||||||
|
+ pse_phy_unlock();
|
||||||
|
|
||||||
|
ethnl_ops_complete(dev);
|
||||||
|
|
||||||
|
@@ -280,9 +285,15 @@ ethnl_set_pse(struct ethnl_req_info *req
|
||||||
|
|
||||||
|
phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PSE_HEADER,
|
||||||
|
info->extack);
|
||||||
|
+
|
||||||
|
+ /* Hold phydev->psec stable against a PSE controller unregister that
|
||||||
|
+ * would detach and free it while it is being dereferenced.
|
||||||
|
+ */
|
||||||
|
+ pse_phy_lock();
|
||||||
|
+
|
||||||
|
ret = ethnl_set_pse_validate(phydev, info);
|
||||||
|
if (ret)
|
||||||
|
- return ret;
|
||||||
|
+ goto out;
|
||||||
|
|
||||||
|
if (tb[ETHTOOL_A_PSE_PRIO]) {
|
||||||
|
unsigned int prio;
|
||||||
|
@@ -290,7 +301,7 @@ ethnl_set_pse(struct ethnl_req_info *req
|
||||||
|
prio = nla_get_u32(tb[ETHTOOL_A_PSE_PRIO]);
|
||||||
|
ret = pse_ethtool_set_prio(phydev->psec, info->extack, prio);
|
||||||
|
if (ret)
|
||||||
|
- return ret;
|
||||||
|
+ goto out;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (tb[ETHTOOL_A_C33_PSE_AVAIL_PW_LIMIT]) {
|
||||||
|
@@ -300,7 +311,7 @@ ethnl_set_pse(struct ethnl_req_info *req
|
||||||
|
ret = pse_ethtool_set_pw_limit(phydev->psec, info->extack,
|
||||||
|
pw_limit);
|
||||||
|
if (ret)
|
||||||
|
- return ret;
|
||||||
|
+ goto out;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* These values are already validated by the ethnl_pse_set_policy */
|
||||||
|
@@ -318,10 +329,11 @@ ethnl_set_pse(struct ethnl_req_info *req
|
||||||
|
*/
|
||||||
|
ret = pse_ethtool_set_config(phydev->psec, info->extack,
|
||||||
|
&config);
|
||||||
|
- if (ret)
|
||||||
|
- return ret;
|
||||||
|
}
|
||||||
|
|
||||||
|
+out:
|
||||||
|
+ pse_phy_unlock();
|
||||||
|
+
|
||||||
|
/* Return errno or zero - PSE has no notification */
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
|||||||
@@ -68,7 +68,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
static DEFINE_MUTEX(pse_list_mutex);
|
static DEFINE_MUTEX(pse_list_mutex);
|
||||||
static LIST_HEAD(pse_controller_list);
|
static LIST_HEAD(pse_controller_list);
|
||||||
static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
|
static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
|
||||||
@@ -1148,6 +1155,15 @@ int pse_controller_register(struct pse_c
|
@@ -1194,6 +1201,15 @@ int pse_controller_register(struct pse_c
|
||||||
blocking_notifier_call_chain(&pse_controller_notifier,
|
blocking_notifier_call_chain(&pse_controller_notifier,
|
||||||
PSE_REGISTERED, pcdev);
|
PSE_REGISTERED, pcdev);
|
||||||
|
|
||||||
@@ -84,7 +84,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(pse_controller_register);
|
EXPORT_SYMBOL_GPL(pse_controller_register);
|
||||||
@@ -1164,6 +1180,8 @@ void pse_controller_unregister(struct ps
|
@@ -1210,6 +1226,8 @@ void pse_controller_unregister(struct ps
|
||||||
pse_flush_pw_ds(pcdev);
|
pse_flush_pw_ds(pcdev);
|
||||||
if (pcdev->irq)
|
if (pcdev->irq)
|
||||||
disable_irq(pcdev->irq);
|
disable_irq(pcdev->irq);
|
||||||
@@ -93,7 +93,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
pse_release_pis(pcdev);
|
pse_release_pis(pcdev);
|
||||||
cancel_work_sync(&pcdev->ntf_work);
|
cancel_work_sync(&pcdev->ntf_work);
|
||||||
kfifo_free(&pcdev->ntf_fifo);
|
kfifo_free(&pcdev->ntf_fifo);
|
||||||
@@ -1286,66 +1304,104 @@ static int pse_set_config_isr(struct pse
|
@@ -1332,66 +1350,104 @@ static int pse_set_config_isr(struct pse
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -229,7 +229,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
/**
|
/**
|
||||||
* devm_pse_irq_helper - Register IRQ based PSE event notifier
|
* devm_pse_irq_helper - Register IRQ based PSE event notifier
|
||||||
* @pcdev: a pointer to the PSE
|
* @pcdev: a pointer to the PSE
|
||||||
@@ -1403,6 +1459,61 @@ int devm_pse_irq_helper(struct pse_contr
|
@@ -1449,6 +1505,61 @@ int devm_pse_irq_helper(struct pse_contr
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(devm_pse_irq_helper);
|
EXPORT_SYMBOL_GPL(devm_pse_irq_helper);
|
||||||
|
|
||||||
|
|||||||
@@ -47,7 +47,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
#include <linux/notifier.h>
|
#include <linux/notifier.h>
|
||||||
#include <linux/of.h>
|
#include <linux/of.h>
|
||||||
#include <linux/phy.h>
|
#include <linux/phy.h>
|
||||||
@@ -704,6 +705,168 @@ static int _pse_pi_delivery_power_sw_pw_
|
@@ -750,6 +751,168 @@ static int _pse_pi_delivery_power_sw_pw_
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -216,7 +216,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
static int pse_pi_enable(struct regulator_dev *rdev)
|
static int pse_pi_enable(struct regulator_dev *rdev)
|
||||||
{
|
{
|
||||||
struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
|
struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
|
||||||
@@ -729,6 +892,7 @@ static int pse_pi_enable(struct regulato
|
@@ -775,6 +938,7 @@ static int pse_pi_enable(struct regulato
|
||||||
pcdev->pi[id].admin_state_enabled = 1;
|
pcdev->pi[id].admin_state_enabled = 1;
|
||||||
ret = 0;
|
ret = 0;
|
||||||
}
|
}
|
||||||
@@ -224,7 +224,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
mutex_unlock(&pcdev->lock);
|
mutex_unlock(&pcdev->lock);
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
@@ -736,6 +900,7 @@ static int pse_pi_enable(struct regulato
|
@@ -782,6 +946,7 @@ static int pse_pi_enable(struct regulato
|
||||||
ret = ops->pi_enable(pcdev, id);
|
ret = ops->pi_enable(pcdev, id);
|
||||||
if (!ret)
|
if (!ret)
|
||||||
pcdev->pi[id].admin_state_enabled = 1;
|
pcdev->pi[id].admin_state_enabled = 1;
|
||||||
@@ -232,7 +232,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
mutex_unlock(&pcdev->lock);
|
mutex_unlock(&pcdev->lock);
|
||||||
|
|
||||||
return ret;
|
return ret;
|
||||||
@@ -759,6 +924,7 @@ static int pse_pi_disable(struct regulat
|
@@ -805,6 +970,7 @@ static int pse_pi_disable(struct regulat
|
||||||
ret = _pse_pi_disable(pcdev, id);
|
ret = _pse_pi_disable(pcdev, id);
|
||||||
if (!ret)
|
if (!ret)
|
||||||
pcdev->pi[id].admin_state_enabled = 0;
|
pcdev->pi[id].admin_state_enabled = 0;
|
||||||
@@ -240,7 +240,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
|
|
||||||
mutex_unlock(&pcdev->lock);
|
mutex_unlock(&pcdev->lock);
|
||||||
return 0;
|
return 0;
|
||||||
@@ -1119,6 +1285,17 @@ int pse_controller_register(struct pse_c
|
@@ -1165,6 +1331,17 @@ int pse_controller_register(struct pse_c
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -258,7 +258,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
/* Each regulator name len is pcdev dev name + 7 char +
|
/* Each regulator name len is pcdev dev name + 7 char +
|
||||||
* int max digit number (10) + 1
|
* int max digit number (10) + 1
|
||||||
*/
|
*/
|
||||||
@@ -1148,6 +1325,19 @@ int pse_controller_register(struct pse_c
|
@@ -1194,6 +1371,19 @@ int pse_controller_register(struct pse_c
|
||||||
if (ret)
|
if (ret)
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
@@ -278,7 +278,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
mutex_lock(&pse_list_mutex);
|
mutex_lock(&pse_list_mutex);
|
||||||
list_add(&pcdev->list, &pse_controller_list);
|
list_add(&pcdev->list, &pse_controller_list);
|
||||||
mutex_unlock(&pse_list_mutex);
|
mutex_unlock(&pse_list_mutex);
|
||||||
@@ -1183,6 +1373,12 @@ void pse_controller_unregister(struct ps
|
@@ -1229,6 +1419,12 @@ void pse_controller_unregister(struct ps
|
||||||
if (pcdev->polling)
|
if (pcdev->polling)
|
||||||
cancel_delayed_work_sync(&pcdev->poll_work);
|
cancel_delayed_work_sync(&pcdev->poll_work);
|
||||||
pse_release_pis(pcdev);
|
pse_release_pis(pcdev);
|
||||||
@@ -291,7 +291,7 @@ Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
|
|||||||
cancel_work_sync(&pcdev->ntf_work);
|
cancel_work_sync(&pcdev->ntf_work);
|
||||||
kfifo_free(&pcdev->ntf_fifo);
|
kfifo_free(&pcdev->ntf_fifo);
|
||||||
mutex_lock(&pse_list_mutex);
|
mutex_lock(&pse_list_mutex);
|
||||||
@@ -1318,12 +1514,21 @@ static void pse_handle_events(struct pse
|
@@ -1364,12 +1560,21 @@ static void pse_handle_events(struct pse
|
||||||
{
|
{
|
||||||
int i;
|
int i;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user