Re: [PATCH] skbuff: remove pointless conditional before kfree_skb()

Eric Dumazet
2012-08-28T14:12:37Z

On Tue, 2012-08-28 at 21:10 +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Remove pointless conditional before kfree_skb().
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> include/linux/skbuff.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 7632c87..0b846d9 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2464,8 +2464,7 @@ static inline void nf_conntrack_get_reasm(struct sk_buff *skb)
> }
> static inline void nf_conntrack_put_reasm(struct sk_buff *skb)
> {
> - if (skb)
> - kfree_skb(skb);
> + kfree_skb(skb);
> }
> #endif
> #ifdef CONFIG_BRIDGE_NETFILTER
>

Its not exactly pointless.

Its a tradeoff between kernel code size, and ability for cpu to predict
a branch in kfree_skb()

This test is in hot path, and therefore this patch can potentially have
a performance impact.

I really think most kfree_skb() calls are done with a non NULL param,
so the branch prediction is good.

But after this patch, things are totally different.

Therefore, I am against it.