[ofa-general] Re: [PATCH] ipoib/cm: compliance fix

Michael S. Tsirkin mst at dev.mellanox.co.il
Wed May 2 15:31:27 PDT 2007


> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCH] ipoib/cm: compliance fix
> 
> thanks, applied
> 
>  > Important enough for -stable?
> 
> I don't think so -- there's no way to hit this in practice right now,
> so I don't think it meets the -stable criteria.
> 
> BTW, looking at the code that happens to be in the patch context:
> 
>  > 	if (!likely(wr_id & IPOIB_CM_RX_UPDATE_MASK)) {
> 
> I think this annotation is unclear and I'm not sure gcc will do what
> is intended here (and I'm not sure what is intended).  Should this be
> 
> 	if (likely(!(wr_id & IPOIB_CM_RX_UPDATE_MASK))) {
> 
> or
> 
> 	if (unlikely(!(wr_id & IPOIB_CM_RX_UPDATE_MASK))) {
> 
> ...seems as if "unlikely" is appropriate.

I expect unlikely to be equivalent: likely means typically == 1,
unlikely means typically == 0, so !likely(x) is equivalent to unlikely(!x).
I did expect gcc to do the right thing here, but go ahead and test if you like.

And I do agree "unlikely" version is more clear.

-- 
MST



More information about the general mailing list