Messages in this thread |  | | Date | Wed, 3 Sep 2025 18:31:20 +0300 | | From | Vladimir Oltean <> | | Subject | Re: [PATCH v2 net 1/2] net: phylink: add lock for serializing concurrent pl->phydev writes with resolver |
| |
On Wed, Sep 03, 2025 at 04:26:35PM +0100, Russell King (Oracle) wrote: > On Wed, Sep 03, 2025 at 06:23:47PM +0300, Vladimir Oltean wrote: > > @@ -2305,6 +2314,7 @@ void phylink_disconnect_phy(struct phylink *pl) > > > > phy = pl->phydev; > > if (phy) { > > + mutex_lock(&pl->phy_lock); > > If we can, I think it would be better to place this a couple of lines > above and move the unlock.
Sorry for potentially misunderstanding, do you mean like this?
mutex_lock(&pl->phy_lock); phy = pl->phydev; if (phy) { mutex_lock(&phy->lock); mutex_lock(&pl->state_mutex); pl->phydev = NULL; pl->phy_enable_tx_lpi = false; pl->mac_tx_clk_stop = false; mutex_unlock(&pl->state_mutex); mutex_unlock(&phy->lock); mutex_unlock(&pl->phy_lock); flush_work(&pl->resolve);
phy_disconnect(phy); } else { mutex_unlock(&pl->phy_lock); }
move the unlock where? because flush_work(&pl->resolve) needs to happen unlocked, otherwise we'll deadlock with phylink_resolve().
Additionally, dereferincing pl->phydev under rtnl_lock() is already safe, and doesn't need the secondary clock.
|  |