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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170928131757.GA23657@kroah.com>
Date:   Thu, 28 Sep 2017 15:17:57 +0200
From:   "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To:     Ruxandra Ioana Radulescu <ruxandra.radulescu@....com>
Cc:     Dan Carpenter <dan.carpenter@...cle.com>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "arnd@...db.de" <arnd@...db.de>,
        "stuyoder@...il.com" <stuyoder@...il.com>,
        Roy Pledge <roy.pledge@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "agraf@...e.de" <agraf@...e.de>,
        Bogdan Purcareata <bogdan.purcareata@....com>,
        Laurentiu Tudor <laurentiu.tudor@....com>
Subject: Re: [PATCH] staging: fsl-mc/dpio: Fix incorrect comparison

On Thu, Sep 28, 2017 at 01:07:48PM +0000, Ruxandra Ioana Radulescu wrote:
> > -----Original Message-----
> > From: Dan Carpenter [mailto:dan.carpenter@...cle.com]
> > Sent: Thursday, September 28, 2017 3:49 PM
> > To: Ruxandra Ioana Radulescu <ruxandra.radulescu@....com>
> > Cc: gregkh@...uxfoundation.org; devel@...verdev.osuosl.org;
> > arnd@...db.de; stuyoder@...il.com; Roy Pledge <roy.pledge@....com>;
> > linux-kernel@...r.kernel.org; agraf@...e.de; Bogdan Purcareata
> > <bogdan.purcareata@....com>; Laurentiu Tudor
> > <laurentiu.tudor@....com>
> > 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