Re: likely/unlikely usage validation

On Tue, May 28, 2024 at 02:03:26PM +0200, Rasmus Villemoes wrote:
> On 28/05/2024 12.37, Mateusz Guzik wrote:
> > I misplaced a closing ) in a patch using unlikely() and broke the
> > comparison, see [1] for context.
> > 
> > The fix was:
> > -               if (unlikely(abs(count + amount)) >= batch) {
> > +               if (unlikely(abs(count + amount) >= batch)) {
> > 
> > Neither kernel build with W=1 nor C=1 (smatch) report the problem.
> > 
> > Given that this compiles fine and for many cases with the bogus usage
> > it also happens to produce the correct result, I have to suspect there
> > are spots in the kernel with the problem.
> > 
> > Either way, sounds like something smatch should be testing for.
> 
> Well, yes. A quick and dirty 'git grep' actually reveals something funny:
> 
> arch/x86/events/intel/core.c:   return unlikely(event->hw.config &
> HSW_IN_TX_CHECKPOINTED) != 0;
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c: if (unlikely(fd_status &
> FM_FD_STAT_RX_ERRORS) != 0) {

The reason why Smatch doesn't find these is because it strips the
"if (unlikely(foo) != 0)" comparisons to just "if (foo)".  The != 0 is a
style choice[1].  If it were >= 0 instead then Smatch would complain.  I
could fix/change this but my motivation is low...  :/

regards,
dan carpenter

[1] https://staticthinking.wordpress.com/2024/02/20/when-to-use-0/