* [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).