[PATCH 2/2] staging: brcm80211: replace asserts close to Mac80211 interface

Gábor Stefanik netrolller.3d at gmail.com
Fri Mar 25 23:54:38 UTC 2011


On Sat, Mar 26, 2011 at 12:19 AM, Greg KH <greg at kroah.com> wrote:
> On Sat, Mar 26, 2011 at 01:19:31AM +0300, Dan Carpenter wrote:
>> On Fri, Mar 25, 2011 at 01:52:57PM +0100, Roland Vossen wrote:
>> > >>
>> > >>>+     if (likely(hw != NULL))
>> > >>>+             wl = hw->priv;
>> > >>
>> > >>This isn't a fast path.  The likely() here is not needed.  It makes the
>> > >>code less readable for no reason.
>> > >
>> > >Good suggestion. Agree.
>> >
>> > Come to think about it a bit better: I would say that the 'likely'
>> > makes the code more readable since it indicates to the reader what
>> > the 'sunny flow' is. Is there a common consensus on the usage of
>> > likely/unlikely ?
>
> The consensus is that is should NEVER show up in a driver.
>
> Really, don't use it, 90% of the time it is used wrong.  That other 10%
> is just lucky.  The compiler knows this stuff better than we do, so let
> it handle it properly.
>
> Again, never use it in a driver.  If you really want to, you had better
> have hard numbers as to why it matters showing a noticable improvement.
>
> thanks,
>
> greg k-h

Plus, IIRC the WARN_ON() macro already wraps its condition into
unlikely(), so unlikely(WARN_ON()) will expand into something like
if(unlikely(unlikely(...))...

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)



More information about the devel mailing list