diff mbox series

[net] lan743x: fix deadlock in lan743x_phy_link_status_change()

Message ID 40e27f76-0ba3-dcef-ee32-a78b9df38b0f@gmail.com (mailing list archive)
State Accepted
Commit ddb826c2c92d461f290a7bab89e7c28696191875
Delegated to: Netdev Maintainers
Headers show
Series [net] lan743x: fix deadlock in lan743x_phy_link_status_change() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 36 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Heiner Kallweit Nov. 24, 2021, 7:16 a.m. UTC
Usage of phy_ethtool_get_link_ksettings() in the link status change
handler isn't needed, and in combination with the referenced change
it results in a deadlock. Simply remove the call and replace it with
direct access to phydev->speed. The duplex argument of 
lan743x_phy_update_flowcontrol() isn't used and can be removed.

Fixes: c10a485c3de5 ("phy: phy_ethtool_ksettings_get: Lock the phy for consistency")
Reported-by: Alessandro B Maurici <abmaurici@gmail.com>
Tested-by: Alessandro B Maurici <abmaurici@gmail.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/microchip/lan743x_main.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Comments

Andrew Lunn Nov. 24, 2021, 3:30 p.m. UTC | #1
On Wed, Nov 24, 2021 at 08:16:25AM +0100, Heiner Kallweit wrote:
> Usage of phy_ethtool_get_link_ksettings() in the link status change
> handler isn't needed, and in combination with the referenced change
> it results in a deadlock. Simply remove the call and replace it with
> direct access to phydev->speed. The duplex argument of 
> lan743x_phy_update_flowcontrol() isn't used and can be removed.
> 
> Fixes: c10a485c3de5 ("phy: phy_ethtool_ksettings_get: Lock the phy for consistency")
> Reported-by: Alessandro B Maurici <abmaurici@gmail.com>
> Tested-by: Alessandro B Maurici <abmaurici@gmail.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Thanks Heiner

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
patchwork-bot+netdevbpf@kernel.org Nov. 25, 2021, 2:50 a.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 24 Nov 2021 08:16:25 +0100 you wrote:
> Usage of phy_ethtool_get_link_ksettings() in the link status change
> handler isn't needed, and in combination with the referenced change
> it results in a deadlock. Simply remove the call and replace it with
> direct access to phydev->speed. The duplex argument of
> lan743x_phy_update_flowcontrol() isn't used and can be removed.
> 
> Fixes: c10a485c3de5 ("phy: phy_ethtool_ksettings_get: Lock the phy for consistency")
> Reported-by: Alessandro B Maurici <abmaurici@gmail.com>
> Tested-by: Alessandro B Maurici <abmaurici@gmail.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> [...]

Here is the summary with links:
  - [net] lan743x: fix deadlock in lan743x_phy_link_status_change()
    https://git.kernel.org/netdev/net/c/ddb826c2c92d

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 4fc97823b..7d7647481 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -914,8 +914,7 @@  static int lan743x_phy_reset(struct lan743x_adapter *adapter)
 }
 
 static void lan743x_phy_update_flowcontrol(struct lan743x_adapter *adapter,
-					   u8 duplex, u16 local_adv,
-					   u16 remote_adv)
+					   u16 local_adv, u16 remote_adv)
 {
 	struct lan743x_phy *phy = &adapter->phy;
 	u8 cap;
@@ -943,7 +942,6 @@  static void lan743x_phy_link_status_change(struct net_device *netdev)
 
 	phy_print_status(phydev);
 	if (phydev->state == PHY_RUNNING) {
-		struct ethtool_link_ksettings ksettings;
 		int remote_advertisement = 0;
 		int local_advertisement = 0;
 
@@ -980,18 +978,14 @@  static void lan743x_phy_link_status_change(struct net_device *netdev)
 		}
 		lan743x_csr_write(adapter, MAC_CR, data);
 
-		memset(&ksettings, 0, sizeof(ksettings));
-		phy_ethtool_get_link_ksettings(netdev, &ksettings);
 		local_advertisement =
 			linkmode_adv_to_mii_adv_t(phydev->advertising);
 		remote_advertisement =
 			linkmode_adv_to_mii_adv_t(phydev->lp_advertising);
 
-		lan743x_phy_update_flowcontrol(adapter,
-					       ksettings.base.duplex,
-					       local_advertisement,
+		lan743x_phy_update_flowcontrol(adapter, local_advertisement,
 					       remote_advertisement);
-		lan743x_ptp_update_latency(adapter, ksettings.base.speed);
+		lan743x_ptp_update_latency(adapter, phydev->speed);
 	}
 }