| 123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101 |
- From 06efc9b8a1360cad83cae6e71558e5458cc1fbf3 Mon Sep 17 00:00:00 2001
- From: Vladimir Oltean <[email protected]>
- Date: Tue, 29 Nov 2022 16:12:14 +0200
- Subject: [PATCH 07/14] net: dpaa2-eth: assign priv->mac after
- dpaa2_mac_connect() call
- There are 2 requirements for correct code:
- - Any time the driver accesses the priv->mac pointer at runtime, it
- either holds NULL to indicate a DPNI-DPNI connection (or unconnected
- DPNI), or a struct dpaa2_mac whose phylink instance was fully
- initialized (created and connected to the PHY). No changes are made to
- priv->mac while it is being used. Currently, rtnl_lock() watches over
- the call to dpaa2_eth_connect_mac(), so it serves the purpose of
- serializing this with all readers of priv->mac.
- - dpaa2_mac_connect() should run unlocked, because inside it are 2
- phylink calls with incompatible locking requirements: phylink_create()
- requires that the rtnl_mutex isn't held, and phylink_fwnode_phy_connect()
- requires that the rtnl_mutex is held. The only way to solve those
- contradictory requirements is to let dpaa2_mac_connect() take
- rtnl_lock() when it needs to.
- To solve both requirements, we need to identify the writer side of the
- priv->mac pointer, which can be wrapped in a mutex private to the driver
- in a future patch. The dpaa2_mac_connect() cannot be part of the writer
- side critical section, because of an AB/BA deadlock with rtnl_lock().
- So the strategy needs to be that where we prepare the DPMAC by calling
- dpaa2_mac_connect(), and only make priv->mac point to it once it's fully
- prepared. This ensures that the writer side critical section has the
- absolute minimum surface it can.
- The reverse strategy is adopted in the dpaa2_eth_disconnect_mac() code
- path. This makes sure that priv->mac is NULL when we start tearing down
- the DPMAC that we disconnected from, and concurrent code will simply not
- see it.
- No locking changes in this patch (concurrent code is still blocked by
- the rtnl_mutex).
- Signed-off-by: Vladimir Oltean <[email protected]>
- Reviewed-by: Ioana Ciornei <[email protected]>
- Tested-by: Ioana Ciornei <[email protected]>
- Signed-off-by: Paolo Abeni <[email protected]>
- ---
- .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 21 +++++++++++--------
- 1 file changed, 12 insertions(+), 9 deletions(-)
- --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
- +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
- @@ -4444,9 +4444,8 @@ static int dpaa2_eth_connect_mac(struct
- err = dpaa2_mac_open(mac);
- if (err)
- goto err_free_mac;
- - priv->mac = mac;
-
- - if (dpaa2_eth_is_type_phy(priv)) {
- + if (dpaa2_mac_is_type_phy(mac)) {
- err = dpaa2_mac_connect(mac);
- if (err && err != -EPROBE_DEFER)
- netdev_err(priv->net_dev, "Error connecting to the MAC endpoint: %pe",
- @@ -4455,11 +4454,12 @@ static int dpaa2_eth_connect_mac(struct
- goto err_close_mac;
- }
-
- + priv->mac = mac;
- +
- return 0;
-
- err_close_mac:
- dpaa2_mac_close(mac);
- - priv->mac = NULL;
- err_free_mac:
- kfree(mac);
- return err;
- @@ -4467,15 +4467,18 @@ err_free_mac:
-
- static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv)
- {
- - if (dpaa2_eth_is_type_phy(priv))
- - dpaa2_mac_disconnect(priv->mac);
- + struct dpaa2_mac *mac = priv->mac;
- +
- + priv->mac = NULL;
-
- - if (!dpaa2_eth_has_mac(priv))
- + if (!mac)
- return;
-
- - dpaa2_mac_close(priv->mac);
- - kfree(priv->mac);
- - priv->mac = NULL;
- + if (dpaa2_mac_is_type_phy(mac))
- + dpaa2_mac_disconnect(mac);
- +
- + dpaa2_mac_close(mac);
- + kfree(mac);
- }
-
- static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
|
