Message ID | 20081125095432.04d73b01@infradead.org |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, 2008-11-25 at 09:54 -0800, Arjan van de Ven wrote: > On Tue, 25 Nov 2008 16:42:03 +0000 > Ben Hutchings <bhutchings@solarflare.com> wrote: > > > On Tue, 2008-11-25 at 08:32 -0800, Arjan van de Ven wrote: > > > From 97a8ab520c378568e172c555a156b469137a3828 Mon Sep 17 00:00:00 > > > 2001 From: Arjan van de Ven <arjan@linux.intel.com> > > > Date: Tue, 25 Nov 2008 08:30:04 -0800 > > > Subject: [PATCH] net: make skb_truesize_bug() call WARN() > > > > > > The truesize message check is important enough to make it print > > > "BUG" to the user console... lets also make it important enough to > > > spit a backtrace/module list etc so that kerneloops.org can track > > > them. [...] > > The bug reports I've seen (with my Debian hat on, not relating to > > sfc) show that configurations that trigger this warning are likely to > > trigger many times over and there's nothing the user can do about > > it. So by all means do make the warning more obvious, but please > > make it WARN_ON_ONCE and then use printk qualified by net_ratelimit(). > > > > if there's no value in those subsequent printk's.. why not just this? [...] I think there may be some value in them: users who want to help debug this can experiment to see which configurations or traffic types do or don't result in invalid truesize. Ben.
From: Ben Hutchings <bhutchings@solarflare.com> Date: Tue, 25 Nov 2008 18:02:19 +0000 > On Tue, 2008-11-25 at 09:54 -0800, Arjan van de Ven wrote: > > On Tue, 25 Nov 2008 16:42:03 +0000 > > Ben Hutchings <bhutchings@solarflare.com> wrote: > > > > > On Tue, 2008-11-25 at 08:32 -0800, Arjan van de Ven wrote: > > > > From 97a8ab520c378568e172c555a156b469137a3828 Mon Sep 17 00:00:00 > > > > 2001 From: Arjan van de Ven <arjan@linux.intel.com> > > > > Date: Tue, 25 Nov 2008 08:30:04 -0800 > > > > Subject: [PATCH] net: make skb_truesize_bug() call WARN() > > > > > > > > The truesize message check is important enough to make it print > > > > "BUG" to the user console... lets also make it important enough to > > > > spit a backtrace/module list etc so that kerneloops.org can track > > > > them. > [...] > > > The bug reports I've seen (with my Debian hat on, not relating to > > > sfc) show that configurations that trigger this warning are likely to > > > trigger many times over and there's nothing the user can do about > > > it. So by all means do make the warning more obvious, but please > > > make it WARN_ON_ONCE and then use printk qualified by net_ratelimit(). > > > > > > > if there's no value in those subsequent printk's.. why not just this? > [...] > > I think there may be some value in them: users who want to help debug > this can experiment to see which configurations or traffic types do or > don't result in invalid truesize. Agreed, netlimit'ing this is the best way to go.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 391be91..1a1f4d1 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -149,7 +149,7 @@ void skb_under_panic(struct sk_buff *skb, int sz, void *here) void skb_truesize_bug(struct sk_buff *skb) { - printk(KERN_ERR "SKB BUG: Invalid truesize (%u) " + WARN_ONCE(1, KERN_ERR "SKB BUG: Invalid truesize (%u) " "len=%u, sizeof(sk_buff)=%Zd\n", skb->truesize, skb->len, sizeof(struct sk_buff)); }