[PATCH net] r8169: don't read interrupt mask register in interrupt handler

[PATCH net] r8169: don't read interrupt mask register in interrupt handler

[Heiner Kallweit]

After the original patch network starts to crash on heavy load.
It's not fully clear why this additional register read has such side
effects, but removing it fixes the issue.

Thanks also to Alex for his contribution and hints.

[0] https://marc.info/?t=155268170400002&r=1&w=2

Fixes: e782410ed237 ("r8169: improve spurious interrupt detection")
Reported-by: VDR User <user.vdr@gmail.com>
Tested-by: VDR User <user.vdr@gmail.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index c29dde064..9dd1cd2c0 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -678,6 +678,7 @@ struct rtl8169_private {
 		struct work_struct work;
 	} wk;
 
+	unsigned irq_enabled:1;
 	unsigned supports_gmii:1;
 	dma_addr_t counters_phys_addr;
 	struct rtl8169_counters *counters;
@@ -1293,6 +1294,7 @@ static void rtl_ack_events(struct rtl8169_private *tp, u16 bits)
 static void rtl_irq_disable(struct rtl8169_private *tp)
 {
 	RTL_W16(tp, IntrMask, 0);
+	tp->irq_enabled = 0;
 }
 
 #define RTL_EVENT_NAPI_RX	(RxOK | RxErr)
@@ -1301,6 +1303,7 @@ static void rtl_irq_disable(struct rtl8169_private *tp)
 
 static void rtl_irq_enable(struct rtl8169_private *tp)
 {
+	tp->irq_enabled = 1;
 	RTL_W16(tp, IntrMask, tp->irq_mask);
 }
 
@@ -6520,9 +6523,8 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 {
 	struct rtl8169_private *tp = dev_instance;
 	u16 status = RTL_R16(tp, IntrStatus);
-	u16 irq_mask = RTL_R16(tp, IntrMask);
 
-	if (status == 0xffff || !(status & irq_mask))
+	if (!tp->irq_enabled || status == 0xffff || !(status & tp->irq_mask))
 		return IRQ_NONE;
 
 	if (unlikely(status & SYSErr)) {
-- 
2.21.0

Re: [PATCH net] r8169: don't read interrupt mask register in interrupt handler

[Alexander Duyck]

On Thu, 2019-03-21 at 21:23 +0100, Heiner Kallweit wrote:
> After the original patch network starts to crash on heavy load.
> It's not fully clear why this additional register read has such side
> effects, but removing it fixes the issue.
> 
> Thanks also to Alex for his contribution and hints.
> 
> [0] https://marc.info/?t=155268170400002&r=1&w=2
> 
> Fixes: e782410ed237 ("r8169: improve spurious interrupt detection")
> Reported-by: VDR User <user.vdr@gmail.com>
> Tested-by: VDR User <user.vdr@gmail.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Looks good to me.

Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>

> ---
>  drivers/net/ethernet/realtek/r8169.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index c29dde064..9dd1cd2c0 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -678,6 +678,7 @@ struct rtl8169_private {
>  		struct work_struct work;
>  	} wk;
>  
> +	unsigned irq_enabled:1;
>  	unsigned supports_gmii:1;
>  	dma_addr_t counters_phys_addr;
>  	struct rtl8169_counters *counters;
> @@ -1293,6 +1294,7 @@ static void rtl_ack_events(struct rtl8169_private *tp, u16 bits)
>  static void rtl_irq_disable(struct rtl8169_private *tp)
>  {
>  	RTL_W16(tp, IntrMask, 0);
> +	tp->irq_enabled = 0;
>  }
>  
>  #define RTL_EVENT_NAPI_RX	(RxOK | RxErr)
> @@ -1301,6 +1303,7 @@ static void rtl_irq_disable(struct rtl8169_private *tp)
>  
>  static void rtl_irq_enable(struct rtl8169_private *tp)
>  {
> +	tp->irq_enabled = 1;
>  	RTL_W16(tp, IntrMask, tp->irq_mask);
>  }
>  
> @@ -6520,9 +6523,8 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
>  {
>  	struct rtl8169_private *tp = dev_instance;
>  	u16 status = RTL_R16(tp, IntrStatus);
> -	u16 irq_mask = RTL_R16(tp, IntrMask);
>  
> -	if (status == 0xffff || !(status & irq_mask))
> +	if (!tp->irq_enabled || status == 0xffff || !(status & tp->irq_mask))
>  		return IRQ_NONE;
>  
>  	if (unlikely(status & SYSErr)) {

Re: [PATCH net] r8169: don't read interrupt mask register in interrupt handler

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Thu, 21 Mar 2019 21:23:14 +0100

> After the original patch network starts to crash on heavy load.
> It's not fully clear why this additional register read has such side
> effects, but removing it fixes the issue.
> 
> Thanks also to Alex for his contribution and hints.
> 
> [0] https://marc.info/?t=155268170400002&r=1&w=2
> 
> Fixes: e782410ed237 ("r8169: improve spurious interrupt detection")
> Reported-by: VDR User <user.vdr@gmail.com>
> Tested-by: VDR User <user.vdr@gmail.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied and queued up for -stable.