On Thu, 23 February 2006 21:10:37 +1100, Herbert Xu wrote:
> On Thu, Feb 23, 2006 at 10:54:46AM +0100, J?rn Engel wrote:
> >
> > Wrt. the binary, you have a point. For source code, my patch does not
> > any new bloat and allows removal of the existing. Lemme do a quick
>
> Well I just did a grep in net/*/*.c and it seems that the number of
> calls to kfree_skb preceded by a NULL check is a small minority. So
> I don't see the point of this as we'll be trading a very small amount
> of source code savings for the bloating (albeit small) of the binary.
For my kernel, there would be 92 removals if the condition at the
price of 135 bytes of extra object code. Some of the removals would
be in modules, so the numbers are not exactly fair.
Another interesting question is: Why is kfree_skb inline in the first
place? After uninlining it, my patch would debloat both source and
object code by a bit:
-rwxr-xr-x 1 joern src 4824435 Feb 23 11:46 vmlinux
12157 bytes gained. Plus a bit more when the 92 conditionals are
removed.
Jörn
--
Optimizations always bust things, because all optimizations are, in
the long haul, a form of cheating, and cheaters eventually get caught.
-- Larry Wall
--- linux-2.6.14-rc3cow/include/linux/skbuff.h~uninline_kfree_skb
2006-02-23 11:40:30.000000000 +0100
+++ linux-2.6.14-rc3cow/include/linux/skbuff.h 2006-02-23 11:41:38.000000000
+0100
@@ -302,6 +302,7 @@ struct sk_buff {
#include <asm/system.h>
+void kfree_skb(struct sk_buff *skb);
extern void __kfree_skb(struct sk_buff *skb);
extern struct sk_buff *__alloc_skb(unsigned int size,
unsigned int __nocast priority, int fclone);
@@ -397,24 +398,6 @@ static inline struct sk_buff *skb_get(st
*/
/**
- * kfree_skb - free an sk_buff
- * @skb: buffer to free
- *
- * Drop a reference to the buffer and free it if the usage count has
- * hit zero.
- */
-static inline void kfree_skb(struct sk_buff *skb)
-{
- if (unlikely(!skb))
- return;
- if (likely(atomic_read(&skb->users) == 1))
- smp_rmb();
- else if (likely(!atomic_dec_and_test(&skb->users)))
- return;
- __kfree_skb(skb);
-}
-
-/**
* skb_cloned - is the buffer a clone
* @skb: buffer to check
*
--- linux-2.6.14-rc3cow/net/core/skbuff.c~uninline_kfree_skb 2006-01-18
14:56:05.000000000 +0100
+++ linux-2.6.14-rc3cow/net/core/skbuff.c 2006-02-23 11:41:56.000000000
+0100
@@ -350,6 +350,24 @@ void __kfree_skb(struct sk_buff *skb)
}
/**
+ * kfree_skb - free an sk_buff
+ * @skb: buffer to free
+ *
+ * Drop a reference to the buffer and free it if the usage count has
+ * hit zero.
+ */
+void kfree_skb(struct sk_buff *skb)
+{
+ if (unlikely(!skb))
+ return;
+ if (likely(atomic_read(&skb->users) == 1))
+ smp_rmb();
+ else if (likely(!atomic_dec_and_test(&skb->users)))
+ return;
+ __kfree_skb(skb);
+}
+
+/**
* skb_clone - duplicate an sk_buff
* @skb: buffer to clone
* @gfp_mask: allocation priority
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
