Re: [PATCH 00/23] Make use of unlikely() more consistently.

Arnd Bergmann
2018-08-31T14:09:54Z

On Fri, Aug 31, 2018 at 12:35 AM Igor Stoppa <igor.stoppa@gmail.com> wrote:
>
> In some cases, checks that are expected to not fail, do not take advantage
> of specifying that the condition being tested is unlikely().
>
> Ex:
>
> #define assert(condition)
> ...
> if (!(condition))
> error_action()
> ...
>
> should be
>
> #define assert(condition)
> ...
> if (unlikely(!(condition)))
> error_action()
> ...

There is a potential that this introduces false-postive -Wmaybe-uninitialized
warnings when CONFIG_PROFILE_ANNOTATED_BRANCHES is
set, since that turns unlikely() into a complex operation that in turn
confuses the compiler so it no longer keeps track of which variables
are initialized or not.

It's possible that none of your patches do that, but one needs to be aware
of the problem, and possibly revert some of your patches if it does cause
warning regressions in drivers that don't actually benefit from the
micro-optimization.

> In other cases, the hint is given twice: once explicitly and once inside
> whatever test is being performed: assert(), BUG_ON(), WARN_ON(), etc.
>
> Ex:
>
> if (unlikely(WARN_ON(condition, message)))
> ...

Removing those is definitely fine.

Arnd