Re: [Xen-devel] net: Detect drivers that reschedule NAPI and exhaust budget

2014-12-20 Thread Eric Dumazet 
On Sat, 2014-12-20 at 17:55 +1100, Herbert Xu wrote:

> -- >8 --
> The commit d75b1ade567ffab085e8adbbdacf0092d10cd09c (net: less
> interrupt masking in NAPI) required drivers to leave poll_list
> empty if the entire budget is consumed.
> 
> We have already had two broken drivers so let's add a check for
> this.
> 
> Signed-off-by: Herbert Xu 
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f411c28..47fdc5c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4620,7 +4620,13 @@ static void net_rx_action(struct softirq_action *h)
>*/
>   napi_gro_flush(n, HZ >= 1000);
>   }
> - list_add_tail(&n->poll_list, &repoll);
> + /* Some drivers may have called napi_schedule
> +  * prior to exhausting their budget.
> +  */
> + if (unlikely(!list_empty(&n->poll_list)))
> + pr_warn("%s: Budget exhausted after 
> napi rescheduled\n", n->dev ? n->dev->name : "backlog");
> + else
> + list_add_tail(&n->poll_list, &repoll);
>   }
>   }
>  
> Thanks,

OK, but could you :
1) use pr_warn_once()
2) split the too long line
pr_warn_once("%s: Budget exhausted after napi rescheduled\n",
 n->dev ? n->dev->name : "backlog");

Thanks Herbert !



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] net: Detect drivers that reschedule NAPI and exhaust budget

2014-12-19 Thread Herbert Xu 
On Fri, Dec 19, 2014 at 09:40:00PM -0500, David Miller wrote:
> From: Eric Dumazet 
> Date: Fri, 19 Dec 2014 17:34:48 -0800
> 
> >> @@ -4620,7 +4620,11 @@ static void net_rx_action(struct softirq_action *h)
> >> */
> >>napi_gro_flush(n, HZ >= 1000);
> >>}
> >> -  list_add_tail(&n->poll_list, &repoll);
> >> +  /* Some drivers may have called napi_schedule
> >> +   * prior to exhausting their budget.
> >> +   */
> >> +  if (!WARN_ON_ONCE(!list_empty(&n->poll_list)))
> >> +  list_add_tail(&n->poll_list, &repoll);
> >>}
> >>}
> >>  
> > 
> > I do not think stack trace will point to the buggy driver ?
> > 
> > IMO it would be better to print a single line with the netdev name ?
> 
> Right, we are already back from the poll routine and will just end
> up seeing the call trace leading to the software interrupt.

Good point Eric.

-- >8 --
The commit d75b1ade567ffab085e8adbbdacf0092d10cd09c (net: less
interrupt masking in NAPI) required drivers to leave poll_list
empty if the entire budget is consumed.

We have already had two broken drivers so let's add a check for
this.

Signed-off-by: Herbert Xu 

diff --git a/net/core/dev.c b/net/core/dev.c
index f411c28..47fdc5c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4620,7 +4620,13 @@ static void net_rx_action(struct softirq_action *h)
 */
napi_gro_flush(n, HZ >= 1000);
}
-   list_add_tail(&n->poll_list, &repoll);
+   /* Some drivers may have called napi_schedule
+* prior to exhausting their budget.
+*/
+   if (unlikely(!list_empty(&n->poll_list)))
+   pr_warn("%s: Budget exhausted after 
napi rescheduled\n", n->dev ? n->dev->name : "backlog");
+   else
+   list_add_tail(&n->poll_list, &repoll);
}
}
 
Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] net: Detect drivers that reschedule NAPI and exhaust budget

2014-12-19 Thread David Miller 
From: Eric Dumazet 
Date: Fri, 19 Dec 2014 17:34:48 -0800

>> @@ -4620,7 +4620,11 @@ static void net_rx_action(struct softirq_action *h)
>>   */
>>  napi_gro_flush(n, HZ >= 1000);
>>  }
>> -list_add_tail(&n->poll_list, &repoll);
>> +/* Some drivers may have called napi_schedule
>> + * prior to exhausting their budget.
>> + */
>> +if (!WARN_ON_ONCE(!list_empty(&n->poll_list)))
>> +list_add_tail(&n->poll_list, &repoll);
>>  }
>>  }
>>  
> 
> I do not think stack trace will point to the buggy driver ?
> 
> IMO it would be better to print a single line with the netdev name ?

Right, we are already back from the poll routine and will just end
up seeing the call trace leading to the software interrupt.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] net: Detect drivers that reschedule NAPI and exhaust budget

2014-12-19 Thread Eric Dumazet 
On Sat, 2014-12-20 at 11:36 +1100, Herbert Xu wrote:
> On Sat, Dec 20, 2014 at 11:23:27AM +1100, Herbert Xu wrote:
> >
> > A similar bug exists in virtio_net.
> 
> In order to detect other drivers doing this we should add something
> like this.
> 
> -- >8 --
> The commit d75b1ade567ffab085e8adbbdacf0092d10cd09c (net: less
> interrupt masking in NAPI) required drivers to leave poll_list
> empty if the entire budget is consumed.
> 
> We have already had two broken drivers so let's add a check for
> this.
>  
> Signed-off-by: Herbert Xu 
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f411c28..88f9725 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4620,7 +4620,11 @@ static void net_rx_action(struct softirq_action *h)
>*/
>   napi_gro_flush(n, HZ >= 1000);
>   }
> - list_add_tail(&n->poll_list, &repoll);
> + /* Some drivers may have called napi_schedule
> +  * prior to exhausting their budget.
> +  */
> + if (!WARN_ON_ONCE(!list_empty(&n->poll_list)))
> + list_add_tail(&n->poll_list, &repoll);
>   }
>   }
>  

I do not think stack trace will point to the buggy driver ?

IMO it would be better to print a single line with the netdev name ?




___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] net: Detect drivers that reschedule NAPI and exhaust budget

2014-12-19 Thread Herbert Xu 
On Sat, Dec 20, 2014 at 11:23:27AM +1100, Herbert Xu wrote:
>
> A similar bug exists in virtio_net.

In order to detect other drivers doing this we should add something
like this.

-- >8 --
The commit d75b1ade567ffab085e8adbbdacf0092d10cd09c (net: less
interrupt masking in NAPI) required drivers to leave poll_list
empty if the entire budget is consumed.

We have already had two broken drivers so let's add a check for
this.
 
Signed-off-by: Herbert Xu 

diff --git a/net/core/dev.c b/net/core/dev.c
index f411c28..88f9725 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4620,7 +4620,11 @@ static void net_rx_action(struct softirq_action *h)
 */
napi_gro_flush(n, HZ >= 1000);
}
-   list_add_tail(&n->poll_list, &repoll);
+   /* Some drivers may have called napi_schedule
+* prior to exhausting their budget.
+*/
+   if (!WARN_ON_ONCE(!list_empty(&n->poll_list)))
+   list_add_tail(&n->poll_list, &repoll);
}
}
 
Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel