[ewg] OFED October 29 meeting summary on OFED 1.3 beta readiness

Jeremy Brown jeremy.brown at qlogic.com
Tue Oct 30 16:40:01 PDT 2007


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




More information about the ewg mailing list