[ofa-general] Re: [ewg] OFED October 29 meeting summary on OFED 1.3 beta readiness

[Jeremy Brown]

On Tue, 2007-10-30 at 16:40 +0200, Tziporet Koren wrote:
>           o Apply patches that fix warning of backport patches - Vlad
>             (Mellanox) (one patch was not applied since we got no answer
>             regarding it)

Yikes, I did drop that on the floor, didn't I? I'm sorry about that.
Here's a reply:

On Thu, 2007-10-25 at 10:05 +0200, Jack Morgenstein wrote: 
> Jeremy,
> 
> Why did you remove the "likely" and "unlikely" macros?
> 
> Isn't the compiler warning just on the missing "!= NULL" ?
> 
> - Jack

It looks like we had something of an internal collision between two
patches. The one we submitted fixes a problem at the likely() unlikely()
macros confuse gcc into thinking that tail could be used before it is
assigned. (The engineer doesn't think gcc is producing better code due
to the use of likely/unlikely here.)

Another change that could be used to fix the issue would be along these
lines:

-     struct sk_buff *tail;
[...]
-     if (likely(skb_len && (tail = skb_peek_tail(&sk->sk_receive_queue))) &&
-         unlikely(skb_tailroom(tail) >= skb_len)) {
-             skb_copy_bits(skb, 0, skb_put(tail, skb_len), skb_len);
-             __kfree_skb(skb);
-             skb = tail;
+     if (likely(skb_len)) {
+             struct sk_buff *tail = skb_peek_tail(&sk->sk_receive_queue);
+             if (likely(tail) && unlikely(skb_tailroom(tail) >= skb_len)) {
+                     skb_copy_bits(skb, 0, skb_put(tail, skb_len), skb_len);
+                     __kfree_skb(skb);
+                     skb = tail;
+             }

Which do you think looks better?

Sorry for the delay!

Jeremy