| Message ID | 1360471749-6177-1-git-send-email-sakiwit@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, 2013-02-09 at 21:49 -0700, Jean Sacren wrote: > Combine skb_over_panic() and skb_under_panic() into skb_panic() and let > the caller specify whether it is skb_over_panic or skb_under_panic. [] > diff --git a/net/core/skbuff.c b/net/core/skbuff.c [] > @@ -104,48 +104,28 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = { [] > -static void skb_over_panic(struct sk_buff *skb, int sz, void *here) > +static void skb_panic(struct sk_buff *skb, unsigned int size, void *addr, > + const char panic[]) > { > - pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n", > - __func__, here, skb->len, sz, skb->head, skb->data, > + pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx " > + "end:%#lx dev:%s\n", Trivia: Don't split the format across multiple lines please. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Joe Perches <joe@perches.com> Date: Sat, 09 Feb 2013 21:11:01 -0800 > > On Sat, 2013-02-09 at 21:49 -0700, Jean Sacren wrote: > > Combine skb_over_panic() and skb_under_panic() into skb_panic() and let > > the caller specify whether it is skb_over_panic or skb_under_panic. > [] > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > [] > > @@ -104,48 +104,28 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = { > [] > > -static void skb_over_panic(struct sk_buff *skb, int sz, void *here) > > +static void skb_panic(struct sk_buff *skb, unsigned int size, void *addr, > > + const char panic[]) > > { > > - pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n", > > - __func__, here, skb->len, sz, skb->head, skb->data, > > + pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx " > > + "end:%#lx dev:%s\n", > > Trivia: > > Don't split the format across multiple lines please. Thank you for reviewing. Honestly I didn't want to split the whole string, but I was in the dilemma of accommodating 80-column convention. I'm ready to respin if the consensus favors one long string. But how long is too long?
On Sat, 2013-02-09 at 22:56 -0700, Jean Sacren wrote: > From: Joe Perches <joe@perches.com> > > Don't split the format across multiple lines please. [] > Honestly I didn't want to split the whole string, but I was in the > dilemma of accommodating 80-column convention. I'm ready to respin if > the consensus favors one long string. But how long is too long? Documentation/CodingStyle Chapter 2: Breaking long lines and strings ... never break user-visible strings such as printk messages, because that breaks the ability to grep for them -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Joe Perches <joe@perches.com> Date: Sat, 09 Feb 2013 22:02:53 -0800 > > On Sat, 2013-02-09 at 22:56 -0700, Jean Sacren wrote: > > From: Joe Perches <joe@perches.com> > > > Don't split the format across multiple lines please. > [] > > Honestly I didn't want to split the whole string, but I was in the > > dilemma of accommodating 80-column convention. I'm ready to respin if > > the consensus favors one long string. But how long is too long? > > Documentation/CodingStyle > > Chapter 2: Breaking long lines and strings > ... > never break user-visible strings such as printk messages, > because that breaks the ability to grep for them I appreciate you for sharing this document with me, but this scenario is unique, as it concerns panic. Under this circumstance, are we able to grep?
On Sat, 2013-02-09 at 23:40 -0700, Jean Sacren wrote: > From: Joe Perches <joe@perches.com> > Date: Sat, 09 Feb 2013 22:02:53 -0800 > > > > On Sat, 2013-02-09 at 22:56 -0700, Jean Sacren wrote: > > > From: Joe Perches <joe@perches.com> > > > > Don't split the format across multiple lines please. > > [] > > > Honestly I didn't want to split the whole string, but I was in the > > > dilemma of accommodating 80-column convention. I'm ready to respin if > > > the consensus favors one long string. But how long is too long? > > > > Documentation/CodingStyle > > > > Chapter 2: Breaking long lines and strings > > ... > > never break user-visible strings such as printk messages, > > because that breaks the ability to grep for them > > I appreciate you for sharing this document with me, but this scenario is > unique, as it concerns panic. Under this circumstance, are we able to > grep? On another machine? In a vm? I don't see why not. Does it matter? Probably not. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 6114c11..e2cc214 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -104,48 +104,28 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = { .get = sock_pipe_buf_get, }; -/* - * Keep out-of-line to prevent kernel bloat. - * __builtin_return_address is not used because it is not always - * reliable. - */ - /** - * skb_over_panic - private function - * @skb: buffer - * @sz: size - * @here: address - * - * Out of line support code for skb_put(). Not user callable. + * skb_panic - private function for out-of-line support + * @skb: buffer + * @size: size + * @addr: address + * @panic: skb_over_panic or skb_under_panic + * + * Out-of-line support for skb_put() and skb_push(). Not user callable. + * Keep out of line to prevent kernel bloat. + * __builtin_return_address is not used because it is not always reliable. */ -static void skb_over_panic(struct sk_buff *skb, int sz, void *here) +static void skb_panic(struct sk_buff *skb, unsigned int size, void *addr, + const char panic[]) { - pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n", - __func__, here, skb->len, sz, skb->head, skb->data, + pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx " + "end:%#lx dev:%s\n", + panic, addr, skb->len, size, skb->head, skb->data, (unsigned long)skb->tail, (unsigned long)skb->end, skb->dev ? skb->dev->name : "<NULL>"); BUG(); } -/** - * skb_under_panic - private function - * @skb: buffer - * @sz: size - * @here: address - * - * Out of line support code for skb_push(). Not user callable. - */ - -static void skb_under_panic(struct sk_buff *skb, int sz, void *here) -{ - pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n", - __func__, here, skb->len, sz, skb->head, skb->data, - (unsigned long)skb->tail, (unsigned long)skb->end, - skb->dev ? skb->dev->name : "<NULL>"); - BUG(); -} - - /* * kmalloc_reserve is a wrapper around kmalloc_node_track_caller that tells * the caller if emergency pfmemalloc reserves are being used. If it is and @@ -1259,12 +1239,13 @@ EXPORT_SYMBOL(skb_pad); */ unsigned char *skb_put(struct sk_buff *skb, unsigned int len) { + const char panic[] = "skb_over_panic"; unsigned char *tmp = skb_tail_pointer(skb); SKB_LINEAR_ASSERT(skb); skb->tail += len; skb->len += len; if (unlikely(skb->tail > skb->end)) - skb_over_panic(skb, len, __builtin_return_address(0)); + skb_panic(skb, len, __builtin_return_address(0), panic); return tmp; } EXPORT_SYMBOL(skb_put); @@ -1280,10 +1261,11 @@ EXPORT_SYMBOL(skb_put); */ unsigned char *skb_push(struct sk_buff *skb, unsigned int len) { + const char panic[] = "skb_under_panic"; skb->data -= len; skb->len += len; if (unlikely(skb->data<skb->head)) - skb_under_panic(skb, len, __builtin_return_address(0)); + skb_panic(skb, len, __builtin_return_address(0), panic); return skb->data; } EXPORT_SYMBOL(skb_push);
Combine skb_over_panic() and skb_under_panic() into skb_panic() and let the caller specify whether it is skb_over_panic or skb_under_panic. In skb_panic() definition, change 'int sz' and 'here' to 'unsigned int size' and 'addr' for clarity, and accommodate the output message. Rewrite kernel doc for skb_panic(). Signed-off-by: Jean Sacren <sakiwit@gmail.com> --- net/core/skbuff.c | 54 ++++++++++++++++++------------------------------------ 1 file changed, 18 insertions(+), 36 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html