Re: [PATCH] staging: fsl-mc/dpio: Fix incorrect comparison

From: gregkh@xxxxxxxxxxxxxxxxxxx
Date: Thu Sep 28 2017 - 09:18:06 EST


On Thu, Sep 28, 2017 at 01:07:48PM +0000, Ruxandra Ioana Radulescu wrote:
> > -----Original Message-----
> > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx]
> > Sent: Thursday, September 28, 2017 3:49 PM
> > To: Ruxandra Ioana Radulescu <ruxandra.radulescu@xxxxxxx>
> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; devel@xxxxxxxxxxxxxxxxxxxx;
> > arnd@xxxxxxxx; stuyoder@xxxxxxxxx; Roy Pledge <roy.pledge@xxxxxxx>;
> > linux-kernel@xxxxxxxxxxxxxxx; agraf@xxxxxxx; Bogdan Purcareata
> > <bogdan.purcareata@xxxxxxx>; Laurentiu Tudor
> > <laurentiu.tudor@xxxxxxx>
> > Subject: Re: [PATCH] staging: fsl-mc/dpio: Fix incorrect comparison
> >
> > On Wed, Sep 27, 2017 at 12:57:28PM -0500, Ioana Radulescu wrote:
> > > diff --git a/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> > b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> > > index f809682..26922fc 100644
> > > --- a/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> > > +++ b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> > > @@ -76,7 +76,7 @@ static inline struct dpaa2_io
> > *service_select_by_cpu(struct dpaa2_io *d,
> > > if (d)
> > > return d;
> > >
> > > - if (unlikely(cpu >= num_possible_cpus()))
> > > + if (unlikely(cpu >= (int)num_possible_cpus()))
> >
> >
> > Drivers shouldn't use likely/unlikley.
>
> I was under the impression it's ok to use them on hotpath
> (and while not entirely obvious, this function is called on
> other drivers' hotpath).

Only use it if you can measure the difference. If you can not, then do
not use it as the compiler and the CPU will guess it better than you
will.

This has been proven many times, something like 80% of our
likely/unlikely usage in the kernel is wrong because of this, see the
work from Andi Kleen many years ago in this area.

So please remove it. Unless you can prove it matters, and if so,
document that.

thanks,

greg k-h