Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180830223429.30051-1-igor.stoppa@huawei.com>
In-Reply-To: <20180830223429.30051-1-igor.stoppa@huawei.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Fri, 31 Aug 2018 16:09:34 +0200
Message-ID: <CAK8P3a2JasKXtXYoV7KsSZt4TwkffsBjrf=LK1HZJx0KFKCcow@mail.gmail.com>
Subject: Re: [PATCH 00/23] Make use of unlikely() more consistently.
To:     igor.stoppa@gmail.com
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        igor.stoppa@huawei.com, Aaro Koskinen <aaro.koskinen@iki.fi>,
        Alasdair Kergon <agk@redhat.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Andrew Jeffery <andrew@aj.id.au>,
        Archit Taneja <architt@codeaurora.org>, asolokha@kb.kras.ru,
        bvanassche@acm.org,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        chien.tin.tung@intel.com, chunkeey@googlemail.com,
        Christoph Lameter <cl@linux.com>,
        David Airlie <airlied@linux.ie>,
        David Daney <ddaney@caviumnetworks.com>,
        Dennis Zhou <dennisszhou@gmail.com>, dima@arista.com,
        Doug Ledford <dledford@redhat.com>,
        "Latif, Faisal" <faisal.latif@intel.com>,
        gregkh <gregkh@linuxfoundation.org>,
        Daode Huang <huangdaode@hisilicon.com>,
        Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>,
        "James E.J. Bottomley" <jejb@linux.vnet.ibm.com>,
        James Hogan <jhogan@kernel.org>,
        Jason Gunthorpe <jgg@ziepe.ca>, ejt@redhat.com,
        Kalle Valo <kvalo@codeaurora.org>,
        Kees Cook <keescook@chromium.org>,
        Larry Finger <Larry.Finger@lwfinger.net>,
        Linus Walleij <linus.walleij@linaro.org>,
        Madalin-Cristian Bucur <madalin.bucur@nxp.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        "Michael S. Tsirkin" <mst@redhat.com>, michal.kazior@tieto.com,
        Mike Snitzer <snitzer@redhat.com>,
        Paul Burton <paul.burton@mips.com>,
        Paul Mackerras <paulus@samba.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Ralf Baechle <ralf@linux-mips.org>,
        Rob Clark <robdclark@gmail.com>, rolandd@cisco.com,
        Salil Mehta <salil.mehta@huawei.com>,
        Shuah Khan <shuah@kernel.org>,
        srivatsa.bhat@linux.vnet.ibm.com, sviau@codeaurora.org,
        Tejun Heo <tj@kernel.org>, yisen.zhuang@huawei.com,
        zijun_hu@htc.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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