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

[David Miller]

From: Tom Lendacky 
Date: Fri, 10 Mar 2017 08:19:37 -0600

> On 3/9/2017 8:31 PM, David Miller wrote:
>> From: Tom Lendacky 
>> 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 
>>> Signed-off-by: Tom Lendacky 
>>
>> 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.

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 



Reported-by: Jeremy Linton 
Signed-off-by: Tom Lendacky 
---
 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

[Tom Lendacky]

On 3/9/2017 8:31 PM, David Miller wrote:

From: Tom Lendacky 
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 
Signed-off-by: Tom Lendacky 


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

[David Miller]

From: Tom Lendacky 
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 
> Signed-off-by: Tom Lendacky 

Applied, thanks.

[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 
Signed-off-by: Tom Lendacky 
---
 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);
}