diff mbox

[v2,08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool

Message ID 20180704081451.7482-4-vladimir_zapolskiy@mentor.com (mailing list archive)
State Accepted
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Vladimir Zapolskiy July 4, 2018, 8:14 a.m. UTC
Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
potentially sleeping") phy_start_aneg() function utilizes a mutex
to serialize changes to phy state, however the helper function is
called in atomic context.

The bug can be reproduced by running "ethtool -r" command, the bug
is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.

Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

Vladimir Zapolskiy July 5, 2018, 6:09 a.m. UTC | #1
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however the helper function is
> called in atomic context.
> 
> The bug can be reproduced by running "ethtool -r" command, the bug
> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
> 
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")

Here is an invalid commit specified, the proper tag is

Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")

--
Best wishes,
Vladimir
Sergei Shtylyov July 5, 2018, 8:27 a.m. UTC | #2
Hello!

On 7/5/2018 9:09 AM, Vladimir Zapolskiy wrote:

>> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
>> potentially sleeping") phy_start_aneg() function utilizes a mutex
>> to serialize changes to phy state, however the helper function is
>> called in atomic context.
>>
>> The bug can be reproduced by running "ethtool -r" command, the bug
>> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
>>
>> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
> 
> Here is an invalid commit specified, the proper tag is
> 
> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")

    I was just going to tell you that. :-)
> --
> Best wishes,
> Vladimir

MBR, Sergei
Sergei Shtylyov July 6, 2018, 7:47 p.m. UTC | #3
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:

> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however the helper function is
> called in atomic context.
> 
> The bug can be reproduced by running "ethtool -r" command, the bug
> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
> 
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[...]

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

   Again, the patch order question...

MBR, Sergei
diff mbox

Patch

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 68f122140966..e7d6d1b6e7d6 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1152,15 +1152,10 @@  static int ravb_set_link_ksettings(struct net_device *ndev,
 
 static int ravb_nway_reset(struct net_device *ndev)
 {
-	struct ravb_private *priv = netdev_priv(ndev);
 	int error = -ENODEV;
-	unsigned long flags;
 
-	if (ndev->phydev) {
-		spin_lock_irqsave(&priv->lock, flags);
+	if (ndev->phydev)
 		error = phy_start_aneg(ndev->phydev);
-		spin_unlock_irqrestore(&priv->lock, flags);
-	}
 
 	return error;
 }