netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] skbuff: create skb_panic() to let caller specify panic
@ 2013-02-10  4:49 Jean Sacren
  2013-02-10  5:11 ` Joe Perches
  0 siblings, 1 reply; 6+ messages in thread
From: Jean Sacren @ 2013-02-10  4:49 UTC (permalink / raw)
  To: netdev

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(-)

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);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbuff: create skb_panic() to let caller specify panic
  2013-02-10  4:49 [PATCH] skbuff: create skb_panic() to let caller specify panic Jean Sacren
@ 2013-02-10  5:11 ` Joe Perches
  2013-02-10  5:56   ` Jean Sacren
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2013-02-10  5:11 UTC (permalink / raw)
  To: Jean Sacren; +Cc: netdev

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbuff: create skb_panic() to let caller specify panic
  2013-02-10  5:11 ` Joe Perches
@ 2013-02-10  5:56   ` Jean Sacren
  2013-02-10  6:02     ` Joe Perches
  0 siblings, 1 reply; 6+ messages in thread
From: Jean Sacren @ 2013-02-10  5:56 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev

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?

-- 
Jean Sacren

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbuff: create skb_panic() to let caller specify panic
  2013-02-10  5:56   ` Jean Sacren
@ 2013-02-10  6:02     ` Joe Perches
  2013-02-10  6:40       ` Jean Sacren
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2013-02-10  6:02 UTC (permalink / raw)
  To: Jean Sacren; +Cc: netdev

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbuff: create skb_panic() to let caller specify panic
  2013-02-10  6:02     ` Joe Perches
@ 2013-02-10  6:40       ` Jean Sacren
  2013-02-10  6:52         ` Joe Perches
  0 siblings, 1 reply; 6+ messages in thread
From: Jean Sacren @ 2013-02-10  6:40 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev

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?

-- 
Jean Sacren

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbuff: create skb_panic() to let caller specify panic
  2013-02-10  6:40       ` Jean Sacren
@ 2013-02-10  6:52         ` Joe Perches
  0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2013-02-10  6:52 UTC (permalink / raw)
  To: Jean Sacren; +Cc: netdev

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-10  6:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-10  4:49 [PATCH] skbuff: create skb_panic() to let caller specify panic Jean Sacren
2013-02-10  5:11 ` Joe Perches
2013-02-10  5:56   ` Jean Sacren
2013-02-10  6:02     ` Joe Perches
2013-02-10  6:40       ` Jean Sacren
2013-02-10  6:52         ` Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).