Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1619020 > unrolled thread

[regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")

Back to article view | Back to linux.kernel

Contents

  [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()") Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-04-07 20:20 +0200
    Re: [regression v4.11] 617f01211baf ("8139too: use  napi_complete_done()") Eric Dumazet <eric.dumazet@gmail.com> - 2017-04-07 20:40 +0200
      Re: [regression v4.11] 617f01211baf ("8139too: use  napi_complete_done()") David Miller <davem@davemloft.net> - 2017-04-07 20:50 +0200
        Re: [regression v4.11] 617f01211baf ("8139too: use  napi_complete_done()") Francois Romieu <romieu@fr.zoreil.com> - 2017-04-08 12:40 +0200
          Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()") tedheadster <tedheadster@gmail.com> - 2017-04-10 16:30 +0200
            Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()") Eric Dumazet <edumazet@google.com> - 2017-04-10 17:00 +0200
      Re: [regression v4.11] 617f01211baf ("8139too: use  napi_complete_done()") Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-04-10 14:20 +0200

#1619020 — [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")

Hi,

My old P3 laptop started to die on me in the middle of larger compile
jobs (using distcc) after v4.11-rc<something>. I bisected the problem
to 617f01211baf ("8139too: use napi_complete_done()").

Unfortunately I wasn't able to capture a full oops as the machine doesn't
have serial and ramoops failed me. I did get one partial oops on vgacon
which showed rtl8139_poll() being involved (EIP was around
_raw_spin_unlock_irqrestore() supposedly), so seems to agree with my
bisect result.

So maybe some kind of nasty thing going between the hard irq and
softirq? Perhaps UP related? I tried to stare at the locking around
rtl8139_poll() for a while but it looked mostly sane to me.

-- 
Ville Syrjälä
Intel OTC

[toc] | [next] | [standalone]

#1619033 — Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")

On Fri, 2017-04-07 at 21:17 +0300, Ville Syrjälä wrote:
> Hi,
> 
> My old P3 laptop started to die on me in the middle of larger compile
> jobs (using distcc) after v4.11-rc<something>. I bisected the problem
> to 617f01211baf ("8139too: use napi_complete_done()").
> 
> Unfortunately I wasn't able to capture a full oops as the machine doesn't
> have serial and ramoops failed me. I did get one partial oops on vgacon
> which showed rtl8139_poll() being involved (EIP was around
> _raw_spin_unlock_irqrestore() supposedly), so seems to agree with my
> bisect result.
> 
> So maybe some kind of nasty thing going between the hard irq and
> softirq? Perhaps UP related? I tried to stare at the locking around
> rtl8139_poll() for a while but it looked mostly sane to me.
> 

Thanks a lot for the detective work, I am so sorry for this !

Could you try the following patch ?

I do not really see what could be wrong, the code should run just fine
on UP.

Thanks.

diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 89631753e79962d91456d93b71929af768917da1..cd2dbec331dd796f5296cd378561b3443f231673 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -2135,11 +2135,12 @@ static int rtl8139_poll(struct napi_struct *napi, int budget)
 	if (likely(RTL_R16(IntrStatus) & RxAckBits))
 		work_done += rtl8139_rx(dev, tp, budget);
 
-	if (work_done < budget && napi_complete_done(napi, work_done)) {
+	if (work_done < budget) {
 		unsigned long flags;
 
 		spin_lock_irqsave(&tp->lock, flags);
-		RTL_W16_F(IntrMask, rtl8139_intr_mask);
+		if (napi_complete_done(napi, work_done))
+			RTL_W16_F(IntrMask, rtl8139_intr_mask);
 		spin_unlock_irqrestore(&tp->lock, flags);
 	}
 	spin_unlock(&tp->rx_lock);

[toc] | [prev] | [next] | [standalone]

#1619043 — Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 07 Apr 2017 11:38:49 -0700

> I do not really see what could be wrong, the code should run just fine
> on UP.

One theory is that the interrupt masking isn't working properly
and interrupts are still arriving and hitting the NAPI state even
when we are actively polling NAPI.

And this problem was masked by the locking done here.

[toc] | [prev] | [next] | [standalone]

#1619259 — Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")

David Miller <davem@davemloft.net> :
[...]
> One theory is that the interrupt masking isn't working properly
> and interrupts are still arriving and hitting the NAPI state even
> when we are actively polling NAPI.
> 
> And this problem was masked by the locking done here.

Yes.

Ville, can you rule out irq sharing between the 8139 and some other
device ? It's a candidate for unexpected interrupt handler invocation
with older pc, even with properly working hardware.

-- 
Ueimor

[toc] | [prev] | [next] | [standalone]

#1619977

On Sat, Apr 8, 2017 at 6:23 AM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> David Miller <davem@davemloft.net> :
> [...]
>> One theory is that the interrupt masking isn't working properly
>> and interrupts are still arriving and hitting the NAPI state even
>> when we are actively polling NAPI.
>>
>> And this problem was masked by the locking done here.
>
> Yes.
>
> Ville, can you rule out irq sharing between the 8139 and some other
> device ? It's a candidate for unexpected interrupt handler invocation
> with older pc, even with properly working hardware.
>

Eric,
  If napi_complete_done() calls could affect drivers on older
hardware, I can test the following:

drivers/net/ethernet/3com/typhoon.c
drivers/net/ethernet/amd/pcnet32.c
drivers/net/ethernet/broadcom/tg3.c
drivers/net/ethernet/dec/tulip/interrupt.c
drivers/net/ethernet/intel/e100.c
drivers/net/ethernet/intel/e1000/e1000_main.c
drivers/net/ethernet/smsc/epic100.c
drivers/net/ethernet/via/via-rhine.c

- Matthew

[toc] | [prev] | [next] | [standalone]

#1619998

On Mon, Apr 10, 2017 at 7:22 AM, tedheadster <tedheadster@gmail.com> wrote:
> On Sat, Apr 8, 2017 at 6:23 AM, Francois Romieu <romieu@fr.zoreil.com> wrote:
>> David Miller <davem@davemloft.net> :
>> [...]
>>> One theory is that the interrupt masking isn't working properly
>>> and interrupts are still arriving and hitting the NAPI state even
>>> when we are actively polling NAPI.
>>>
>>> And this problem was masked by the locking done here.
>>
>> Yes.
>>
>> Ville, can you rule out irq sharing between the 8139 and some other
>> device ? It's a candidate for unexpected interrupt handler invocation
>> with older pc, even with properly working hardware.
>>
>
> Eric,
>   If napi_complete_done() calls could affect drivers on older
> hardware, I can test the following:
>
> drivers/net/ethernet/3com/typhoon.c
> drivers/net/ethernet/amd/pcnet32.c
> drivers/net/ethernet/broadcom/tg3.c
> drivers/net/ethernet/dec/tulip/interrupt.c
> drivers/net/ethernet/intel/e100.c
> drivers/net/ethernet/intel/e1000/e1000_main.c
> drivers/net/ethernet/smsc/epic100.c
> drivers/net/ethernet/via/via-rhine.c

That would be great, thanks Matthew.

[toc] | [prev] | [next] | [standalone]

#1619861 — Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")

On Fri, Apr 07, 2017 at 11:38:49AM -0700, Eric Dumazet wrote:
> On Fri, 2017-04-07 at 21:17 +0300, Ville Syrjälä wrote:
> > Hi,
> > 
> > My old P3 laptop started to die on me in the middle of larger compile
> > jobs (using distcc) after v4.11-rc<something>. I bisected the problem
> > to 617f01211baf ("8139too: use napi_complete_done()").
> > 
> > Unfortunately I wasn't able to capture a full oops as the machine doesn't
> > have serial and ramoops failed me. I did get one partial oops on vgacon
> > which showed rtl8139_poll() being involved (EIP was around
> > _raw_spin_unlock_irqrestore() supposedly), so seems to agree with my
> > bisect result.
> > 
> > So maybe some kind of nasty thing going between the hard irq and
> > softirq? Perhaps UP related? I tried to stare at the locking around
> > rtl8139_poll() for a while but it looked mostly sane to me.
> > 
> 
> Thanks a lot for the detective work, I am so sorry for this !
> 
> Could you try the following patch ?
> 
> I do not really see what could be wrong, the code should run just fine
> on UP.
> 
> Thanks.
> 
> diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
> index 89631753e79962d91456d93b71929af768917da1..cd2dbec331dd796f5296cd378561b3443f231673 100644
> --- a/drivers/net/ethernet/realtek/8139too.c
> +++ b/drivers/net/ethernet/realtek/8139too.c
> @@ -2135,11 +2135,12 @@ static int rtl8139_poll(struct napi_struct *napi, int budget)
>  	if (likely(RTL_R16(IntrStatus) & RxAckBits))
>  		work_done += rtl8139_rx(dev, tp, budget);
>  
> -	if (work_done < budget && napi_complete_done(napi, work_done)) {
> +	if (work_done < budget) {
>  		unsigned long flags;
>  
>  		spin_lock_irqsave(&tp->lock, flags);
> -		RTL_W16_F(IntrMask, rtl8139_intr_mask);
> +		if (napi_complete_done(napi, work_done))
> +			RTL_W16_F(IntrMask, rtl8139_intr_mask);
>  		spin_unlock_irqrestore(&tp->lock, flags);
>  	}
>  	spin_unlock(&tp->rx_lock);
> 
> 

Yep, that patch does appear to make it stable again.

Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

-- 
Ville Syrjälä
Intel OTC

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web