[PATCH net] eth: alx: take rtnl_lock on resume

[PATCH net] eth: alx: take rtnl_lock on resume

[Jakub Kicinski]

Zbynek reports that alx trips an rtnl assertion on resume:

 RTNL: assertion failed at net/core/dev.c (2891)
 RIP: 0010:netif_set_real_num_tx_queues+0x1ac/0x1c0
 Call Trace:
  <TASK>
  __alx_open+0x230/0x570 [alx]
  alx_resume+0x54/0x80 [alx]
  ? pci_legacy_resume+0x80/0x80
  dpm_run_callback+0x4a/0x150
  device_resume+0x8b/0x190
  async_resume+0x19/0x30
  async_run_entry_fn+0x30/0x130
  process_one_work+0x1e5/0x3b0

indeed the driver does not hold rtnl_lock during its internal close
and re-open functions during suspend/resume. Note that this is not
a huge bug as the driver implements its own locking, and does not
implement changing the number of queues, but we need to silence
the splat.

Fixes: 4a5fe57e7751 ("alx: use fine-grained locking instead of RTNL")
Reported-and-tested-by: Zbynek Michl <zbynek.michl@gmail.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: chris.snook@gmail.com
CC: dossche.niels@gmail.com
CC: johannes@sipsolutions.net
---
 drivers/net/ethernet/atheros/alx/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index a89b93cb4e26..d5939586c82e 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1912,11 +1912,14 @@ static int alx_suspend(struct device *dev)
 
 	if (!netif_running(alx->dev))
 		return 0;
+
+	rtnl_lock();
 	netif_device_detach(alx->dev);
 
 	mutex_lock(&alx->mtx);
 	__alx_stop(alx);
 	mutex_unlock(&alx->mtx);
+	rtnl_unlock();
 
 	return 0;
 }
@@ -1927,6 +1930,7 @@ static int alx_resume(struct device *dev)
 	struct alx_hw *hw = &alx->hw;
 	int err;
 
+	rtnl_lock();
 	mutex_lock(&alx->mtx);
 	alx_reset_phy(hw);
 
@@ -1943,6 +1947,7 @@ static int alx_resume(struct device *dev)
 
 unlock:
 	mutex_unlock(&alx->mtx);
+	rtnl_unlock();
 	return err;
 }
 
-- 
2.37.3

Re: [PATCH net] eth: alx: take rtnl_lock on resume

[Niels Dossche]

On 9/28/22 20:12, Jakub Kicinski wrote:
> Zbynek reports that alx trips an rtnl assertion on resume:
> 
>  RTNL: assertion failed at net/core/dev.c (2891)
>  RIP: 0010:netif_set_real_num_tx_queues+0x1ac/0x1c0
>  Call Trace:
>   <TASK>
>   __alx_open+0x230/0x570 [alx]
>   alx_resume+0x54/0x80 [alx]
>   ? pci_legacy_resume+0x80/0x80
>   dpm_run_callback+0x4a/0x150
>   device_resume+0x8b/0x190
>   async_resume+0x19/0x30
>   async_run_entry_fn+0x30/0x130
>   process_one_work+0x1e5/0x3b0
> 
> indeed the driver does not hold rtnl_lock during its internal close
> and re-open functions during suspend/resume. Note that this is not
> a huge bug as the driver implements its own locking, and does not
> implement changing the number of queues, but we need to silence
> the splat.
> 
> Fixes: 4a5fe57e7751 ("alx: use fine-grained locking instead of RTNL")
> Reported-and-tested-by: Zbynek Michl <zbynek.michl@gmail.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: chris.snook@gmail.com
> CC: dossche.niels@gmail.com
> CC: johannes@sipsolutions.net
> ---
>  drivers/net/ethernet/atheros/alx/main.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
> index a89b93cb4e26..d5939586c82e 100644
> --- a/drivers/net/ethernet/atheros/alx/main.c
> +++ b/drivers/net/ethernet/atheros/alx/main.c
> @@ -1912,11 +1912,14 @@ static int alx_suspend(struct device *dev)
>  
>  	if (!netif_running(alx->dev))
>  		return 0;
> +
> +	rtnl_lock();
>  	netif_device_detach(alx->dev);
>  
>  	mutex_lock(&alx->mtx);
>  	__alx_stop(alx);
>  	mutex_unlock(&alx->mtx);
> +	rtnl_unlock();
>  
>  	return 0;
>  }
> @@ -1927,6 +1930,7 @@ static int alx_resume(struct device *dev)
>  	struct alx_hw *hw = &alx->hw;
>  	int err;
>  
> +	rtnl_lock();
>  	mutex_lock(&alx->mtx);
>  	alx_reset_phy(hw);
>  
> @@ -1943,6 +1947,7 @@ static int alx_resume(struct device *dev)
>  
>  unlock:
>  	mutex_unlock(&alx->mtx);
> +	rtnl_unlock();
>  	return err;
>  }
>  

Reviewed-by: Niels Dossche <dossche.niels@gmail.com>

Re: [PATCH net] eth: alx: take rtnl_lock on resume

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 28 Sep 2022 11:12:36 -0700 you wrote:
> Zbynek reports that alx trips an rtnl assertion on resume:
> 
>  RTNL: assertion failed at net/core/dev.c (2891)
>  RIP: 0010:netif_set_real_num_tx_queues+0x1ac/0x1c0
>  Call Trace:
>   <TASK>
>   __alx_open+0x230/0x570 [alx]
>   alx_resume+0x54/0x80 [alx]
>   ? pci_legacy_resume+0x80/0x80
>   dpm_run_callback+0x4a/0x150
>   device_resume+0x8b/0x190
>   async_resume+0x19/0x30
>   async_run_entry_fn+0x30/0x130
>   process_one_work+0x1e5/0x3b0
> 
> [...]

Here is the summary with links:
  - [net] eth: alx: take rtnl_lock on resume
    https://git.kernel.org/netdev/net/c/6ad1c94e1e7e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html