[PATCH net] amd-xgbe: Enable IRQs only if napi_complete_done() is true

[PATCH net] amd-xgbe: Enable IRQs only if napi_complete_done() is true

[Tom Lendacky]

Depending on the hardware, the amd-xgbe driver may use disable_irq_nosync()
and enable_irq() when an interrupt is received to process Rx packets. If
the napi_complete_done() return value isn't checked an unbalanced enable
for the IRQ could result, generating a warning stack trace.

Update the driver to only enable interrupts if napi_complete_done() returns
true.

Reported-by: Jeremy Linton <jeremy.linton@arm.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 248f60d..ffea985 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -2272,10 +2272,7 @@ static int xgbe_one_poll(struct napi_struct *napi, int budget)
 	processed = xgbe_rx_poll(channel, budget);
 
 	/* If we processed everything, we are done */
-	if (processed < budget) {
-		/* Turn off polling */
-		napi_complete_done(napi, processed);
-
+	if ((processed < budget) && napi_complete_done(napi, processed)) {
 		/* Enable Tx and Rx interrupts */
 		if (pdata->channel_irq_mode)
 			xgbe_enable_rx_tx_int(pdata, channel);
@@ -2317,10 +2314,7 @@ static int xgbe_all_poll(struct napi_struct *napi, int budget)
 	} while ((processed < budget) && (processed != last_processed));
 
 	/* If we processed everything, we are done */
-	if (processed < budget) {
-		/* Turn off polling */
-		napi_complete_done(napi, processed);
-
+	if ((processed < budget) && napi_complete_done(napi, processed)) {
 		/* Enable Tx and Rx interrupts */
 		xgbe_enable_rx_tx_ints(pdata);
 	}

Re: [PATCH net] amd-xgbe: Enable IRQs only if napi_complete_done() is true

[David Miller]

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Thu, 9 Mar 2017 17:48:23 -0600

> Depending on the hardware, the amd-xgbe driver may use disable_irq_nosync()
> and enable_irq() when an interrupt is received to process Rx packets. If
> the napi_complete_done() return value isn't checked an unbalanced enable
> for the IRQ could result, generating a warning stack trace.
> 
> Update the driver to only enable interrupts if napi_complete_done() returns
> true.
> 
> Reported-by: Jeremy Linton <jeremy.linton@arm.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Applied, thanks.

Re: [PATCH net] amd-xgbe: Enable IRQs only if napi_complete_done() is true

[Tom Lendacky]

On 3/9/2017 8:31 PM, David Miller wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Thu, 9 Mar 2017 17:48:23 -0600
>
>> Depending on the hardware, the amd-xgbe driver may use disable_irq_nosync()
>> and enable_irq() when an interrupt is received to process Rx packets. If
>> the napi_complete_done() return value isn't checked an unbalanced enable
>> for the IRQ could result, generating a warning stack trace.
>>
>> Update the driver to only enable interrupts if napi_complete_done() returns
>> true.
>>
>> Reported-by: Jeremy Linton <jeremy.linton@arm.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>
> Applied, thanks.

Thanks David!  The change to napi_complete_done() from void to bool
occurred in 4.10, can you queue this fix up against 4.10 stable?

Thanks,
Tom

>

Re: [PATCH net] amd-xgbe: Enable IRQs only if napi_complete_done() is true

[Jeremy Linton]

On 03/09/2017 05:48 PM, Tom Lendacky wrote:
> Depending on the hardware, the amd-xgbe driver may use disable_irq_nosync()
> and enable_irq() when an interrupt is received to process Rx packets. If
> the napi_complete_done() return value isn't checked an unbalanced enable
> for the IRQ could result, generating a warning stack trace.
>
> Update the driver to only enable interrupts if napi_complete_done() returns
> true.

I've been running this for a few hours now and haven't seen the warning. 
So it appears to be fixed. Thanks!

I guess Dave M picked it up already, but I will toss this in anyway.


Tested-by: Jeremy Linton <jeremy.linton@arm.com>

>
> Reported-by: Jeremy Linton <jeremy.linton@arm.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  drivers/net/ethernet/amd/xgbe/xgbe-drv.c |   10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> index 248f60d..ffea985 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> @@ -2272,10 +2272,7 @@ static int xgbe_one_poll(struct napi_struct *napi, int budget)
>  	processed = xgbe_rx_poll(channel, budget);
>
>  	/* If we processed everything, we are done */
> -	if (processed < budget) {
> -		/* Turn off polling */
> -		napi_complete_done(napi, processed);
> -
> +	if ((processed < budget) && napi_complete_done(napi, processed)) {
>  		/* Enable Tx and Rx interrupts */
>  		if (pdata->channel_irq_mode)
>  			xgbe_enable_rx_tx_int(pdata, channel);
> @@ -2317,10 +2314,7 @@ static int xgbe_all_poll(struct napi_struct *napi, int budget)
>  	} while ((processed < budget) && (processed != last_processed));
>
>  	/* If we processed everything, we are done */
> -	if (processed < budget) {
> -		/* Turn off polling */
> -		napi_complete_done(napi, processed);
> -
> +	if ((processed < budget) && napi_complete_done(napi, processed)) {
>  		/* Enable Tx and Rx interrupts */
>  		xgbe_enable_rx_tx_ints(pdata);
>  	}
>

Re: [PATCH net] amd-xgbe: Enable IRQs only if napi_complete_done() is true

[David Miller]

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Fri, 10 Mar 2017 08:19:37 -0600

> On 3/9/2017 8:31 PM, David Miller wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>> Date: Thu, 9 Mar 2017 17:48:23 -0600
>>
>>> Depending on the hardware, the amd-xgbe driver may use
>>> disable_irq_nosync()
>>> and enable_irq() when an interrupt is received to process Rx
>>> packets. If
>>> the napi_complete_done() return value isn't checked an unbalanced
>>> enable
>>> for the IRQ could result, generating a warning stack trace.
>>>
>>> Update the driver to only enable interrupts if napi_complete_done()
>>> returns
>>> true.
>>>
>>> Reported-by: Jeremy Linton <jeremy.linton@arm.com>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> Applied, thanks.
> 
> Thanks David!  The change to napi_complete_done() from void to bool
> occurred in 4.10, can you queue this fix up against 4.10 stable?

Sure, done.