Skip to contentSkip to conversation

skbuff: remove old NET_CALLER macro

PATCH In review Stephen Hemminger 3 participants 4 messages Apr 19, 2005 – Apr 20, 2005 1 message not in the archive

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

PATCH 1/1 skbuff: remove old NET_CALLER macro

Kill NET_CALLER it causes warnings from sparse and is no longer necessary.

Signed-off-by Stephen Hemminger <[email protected]>
include/linux/skbuff.h +0 -6
diff -urNp -X dontdiff ck-2.6.11/include/linux/skbuff.h net-2.6.11/include/linux/skbuff.h
@@ -83,12 +83,6 @@
  *	Any questions? No questions, good. 		--ANK
  */
 
-#ifdef __i386__
-#define NET_CALLER(arg) (*(((void **)&arg) - 1))
-#else
-#define NET_CALLER(arg) __builtin_return_address(0)
-#endif
-
 struct net_device;
 
 #ifdef CONFIG_NETFILTER
net/core/skbuff.c +6 -4
diff -urNp -X dontdiff ck-2.6.11/net/core/skbuff.c net-2.6.11/net/core/skbuff.c
@@ -275,9 +275,10 @@ void kfree_skbmem(struct sk_buff *skb)
 
 void __kfree_skb(struct sk_buff *skb)
 {
-	if (skb->list) {
+	if (unlikely(skb->list)) {
 	 	printk(KERN_WARNING "Warning: kfree_skb passed an skb still "
-		       "on a list (from %p).\n", NET_CALLER(skb));
+		       "on a list (from %p).\n", 
+			__builtin_return_address(0));
 		BUG();
 	}
 

@@ -285,10 +286,11 @@ void __kfree_skb(struct sk_buff *skb)
 #ifdef CONFIG_XFRM
 	secpath_put(skb->sp);
 #endif
-	if(skb->destructor) {
+	if (unlikely(skb->destructor)) {
 		if (in_irq())
 			printk(KERN_WARNING "Warning: kfree_skb on "
-					    "hard IRQ %p\n", NET_CALLER(skb));
+					    "hard IRQ %p\n",
+			__builtin_return_address(0));
 		skb->destructor(skb);
 	}
 #ifdef CONFIG_NETFILTER
#1 lore raw

net/core/skbuff.c 2005-04-19 17:31:38.000000000 +1000 void kfree_skbmem(struct sk_buff *skb) · line 282

Quoted 13 lines · Stephen Hemminger
net/core/skbuff.c 2005-04-19 17:31:38.000000000 +1000 renamed from net/core/skbuff.c 2005-04-19 17:07:13.000000000 +1000 +3 -2
@@ -275,9 +275,10 @@ void kfree_skbmem(struct sk_buff *skb)
 
 void __kfree_skb(struct sk_buff *skb)
 {
-       if (skb->list) {
+       if (unlikely(skb->list)) {
               printk(KERN_WARNING "Warning: kfree_skb passed an skb still "
-                      "on a list (from %p).\n", NET_CALLER(skb));
+                      "on a list (from %p).\n", 
+                       __builtin_return_address(0));
               BUG();

Why not turn this if block into a BUG_ON? The return address will be printed by BUG anyway.

net/core/skbuff.c 2005-04-19 17:31:38.000000000 +1000 void __kfree_skb(struct sk_buff *skb) · line 289

Quoted 6 lines
net/core/skbuff.c 2005-04-19 17:31:38.000000000 +1000 renamed from net/core/skbuff.c 2005-04-19 17:07:13.000000000 +1000 +1 -1
@@ -285,10 +286,11 @@ void __kfree_skb(struct sk_buff *skb)
 #ifdef CONFIG_XFRM
       secpath_put(skb->sp);
 #endif
-       if(skb->destructor) {
+       if (unlikely(skb->destructor)) {

Are you sure that this is the less common case?

net/core/skbuff.c 2005-04-19 17:31:38.000000000 +1000 void __kfree_skb(struct sk_buff *skb) · line 293

Quoted 5 lines
net/core/skbuff.c 2005-04-19 17:31:38.000000000 +1000 renamed from net/core/skbuff.c 2005-04-19 17:07:13.000000000 +1000 +2 -1
               if (in_irq())
                       printk(KERN_WARNING "Warning: kfree_skb on "
-                                           "hard IRQ %p\n", NET_CALLER(skb));
+                                           "hard IRQ %p\n",
+                       __builtin_return_address(0));

Similarly, This can be turned into a WARN_ON.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#2 lore raw
Quoted 2 lines · On Tue, 19 Apr 2005 18:04:43 +1000 Herbert Xu <[email protected]>…

Why not turn this if block into a BUG_ON? The return address will be printed by BUG anyway.

Also, __builtin_return_address(0) does not work reliably on some platforms, notably MIPS. However, you are making a straight transformation so you're not making things any worse than before.

Eventually, this belongs in linux/compiler.h as "function_caller()" or something like that.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 2 lines · Stephen Hemminger

Here is a revised alternative that uses BUG_ON/WARN_ON to eliminate NET_CALLER.

Looks good. Thanks Stephen.

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt