[RFC PATCH] net: stmmac: remove rtnl unlock/lock sequence from stmmac_resume

[RFC PATCH] net: stmmac: remove rtnl unlock/lock sequence from stmmac_resume

[Edmundo Carmona Antoranz]

90702dcd19c ("net: stmmac: fix MAC not working when system resume back
with WoL active") introduced a section of code in stmmac_resume that
is protected with rtnl_lock/rtnl_unlock. However, the call to
rtnl_unlock() is followed by an already existing call to rtnl_lock().

Instead of unlock/lock, allow the prexisting lock to continue.

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index eb3b7bf771d7..5e69380027af 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7263,9 +7263,7 @@ int stmmac_resume(struct device *dev)
 		if (device_may_wakeup(priv->device))
 			phylink_speed_up(priv->phylink);
 	}
-	rtnl_unlock();
 
-	rtnl_lock();
 	mutex_lock(&priv->lock);
 
 	stmmac_reset_queues_param(priv);
-- 
2.33.0

Is this a valid patch? Perhaps the lock is released in the middle
on purpose but I would assume it was just an little oversight.

RE: [RFC PATCH] net: stmmac: remove rtnl unlock/lock sequence from stmmac_resume

[Joakim Zhang]

> -----Original Message-----
> From: Edmundo Carmona Antoranz <eantoranz@gmail.com>
> Sent: 2021年10月14日 20:44
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: kernel-janitors@vger.kernel.org; Edmundo Carmona Antoranz
> <eantoranz@gmail.com>
> Subject: [RFC PATCH] net: stmmac: remove rtnl unlock/lock sequence from
> stmmac_resume
> 
> 90702dcd19c ("net: stmmac: fix MAC not working when system resume back
> with WoL active") introduced a section of code in stmmac_resume that is
> protected with rtnl_lock/rtnl_unlock. However, the call to
> rtnl_unlock() is followed by an already existing call to rtnl_lock().
> 
> Instead of unlock/lock, allow the prexisting lock to continue.
> 
> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index eb3b7bf771d7..5e69380027af 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7263,9 +7263,7 @@ int stmmac_resume(struct device *dev)
>  		if (device_may_wakeup(priv->device))
>  			phylink_speed_up(priv->phylink);
>  	}
> -	rtnl_unlock();
> 
> -	rtnl_lock();
>  	mutex_lock(&priv->lock);
> 
>  	stmmac_reset_queues_param(priv);
> --
> 2.33.0
> 
> Is this a valid patch? Perhaps the lock is released in the middle on purpose but I
> would assume it was just an little oversight.

Hi Edmundo,

When I prepared this fix, I also notice this unlock/lock. Finally, I decide to do this way, since I just want to
use rtnl lock to protect what I changed, that means this code change is not belong to what rtnl lock to protect next.
Such as, someone can add code after this rtnl_unlock and before rtnl_lock...

Emm, if you want to send this patch to fix it, I am also fine, please may do some stress test, thanks.

Best Regards,
Joakim Zhang