[PATCH net-next 12/12] net: dpaa2-mac: move rtnl_lock() only around phylink_{,dis}connect_phy()

From: Vladimir Oltean
Date: Tue Nov 29 2022 - 09:14:45 EST


After the introduction of a private mac_lock that serializes access to
priv->mac (and port_priv->mac in the switch), the only remaining purpose
of rtnl_lock() is to satisfy the locking requirements of
phylink_fwnode_phy_connect() and phylink_disconnect_phy().

But the functions these live in, dpaa2_mac_connect() and
dpaa2_mac_disconnect(), have contradictory locking requirements.
While phylink_fwnode_phy_connect() wants rtnl_lock() to be held,
phylink_create() wants it to not be held.

Move the rtnl_lock() from top-level (in the dpaa2-eth and dpaa2-switch
drivers) to only surround the phylink calls that require it, in the
dpaa2-mac library code.

This is possible because dpaa2_mac_connect() and dpaa2_mac_disconnect()
run unlocked, and there isn't any danger of an AB/BA deadlock between
the rtnl_mutex and other private locks.

Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
---
drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 4 ----
drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 5 +++++
drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 4 ----
3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 3ed54c147e98..0c35abb7d065 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -4715,7 +4715,6 @@ static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
dpaa2_eth_set_mac_addr(netdev_priv(net_dev));
dpaa2_eth_update_tx_fqids(priv);

- rtnl_lock();
/* We can avoid locking because the "endpoint changed" IRQ
* handler is the only one who changes priv->mac at runtime,
* so we are not racing with anyone.
@@ -4725,7 +4724,6 @@ static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
dpaa2_eth_disconnect_mac(priv);
else
dpaa2_eth_connect_mac(priv);
- rtnl_unlock();
}

return IRQ_HANDLED;
@@ -5045,9 +5043,7 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
else
fsl_mc_free_irqs(ls_dev);

- rtnl_lock();
dpaa2_eth_disconnect_mac(priv);
- rtnl_unlock();
dpaa2_eth_free_rings(priv);
free_percpu(priv->fd);
free_percpu(priv->sgt_cache);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 9d1e7026eaef..8ba4ea4adeb3 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -431,7 +431,9 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
}
mac->phylink = phylink;

+ rtnl_lock();
err = phylink_fwnode_phy_connect(mac->phylink, dpmac_node, 0);
+ rtnl_unlock();
if (err) {
netdev_err(net_dev, "phylink_fwnode_phy_connect() = %d\n", err);
goto err_phylink_destroy;
@@ -449,7 +451,10 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)

void dpaa2_mac_disconnect(struct dpaa2_mac *mac)
{
+ rtnl_lock();
phylink_disconnect_phy(mac->phylink);
+ rtnl_unlock();
+
phylink_destroy(mac->phylink);
dpaa2_pcs_destroy(mac);
of_phy_put(mac->serdes_phy);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 0472e24191ad..f4ae4289c41a 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -1529,7 +1529,6 @@ static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg)
}

if (status & DPSW_IRQ_EVENT_ENDPOINT_CHANGED) {
- rtnl_lock();
/* We can avoid locking because the "endpoint changed" IRQ
* handler is the only one who changes priv->mac at runtime,
* so we are not racing with anyone.
@@ -1539,7 +1538,6 @@ static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg)
dpaa2_switch_port_disconnect_mac(port_priv);
else
dpaa2_switch_port_connect_mac(port_priv);
- rtnl_unlock();
}

out:
@@ -2957,9 +2955,7 @@ static void dpaa2_switch_remove_port(struct ethsw_core *ethsw,
{
struct ethsw_port_priv *port_priv = ethsw->ports[port_idx];

- rtnl_lock();
dpaa2_switch_port_disconnect_mac(port_priv);
- rtnl_unlock();
free_netdev(port_priv->netdev);
ethsw->ports[port_idx] = NULL;
}
--
2.34.1