From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752910AbcBEKwr (ORCPT ); Fri, 5 Feb 2016 05:52:47 -0500 Received: from mout.kundenserver.de ([212.227.17.24]:64356 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750874AbcBEKwo (ORCPT ); Fri, 5 Feb 2016 05:52:44 -0500 From: Arnd Bergmann To: Andrew Morton Cc: Andrzej Hajda , Bartlomiej Zolnierkiewicz , Marek Szyprowski , open list , Bob Peterson Subject: Re: [PATCH v3] err.h: allow IS_ERR_VALUE to handle properly more types Date: Fri, 05 Feb 2016 11:52:29 +0100 Message-ID: <2046663.fHIlWH1ph1@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20160204105931.7422c17d0bd1b92a387d97c9@linux-foundation.org> References: <20160202163350.f7d42f4b97f48756f3900e9a@linux-foundation.org> <13351313.c4ZqBbQEld@wuerfel> <20160204105931.7422c17d0bd1b92a387d97c9@linux-foundation.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:l8KeTLn5Pe6IPohw+LoXU2G9xMU2alZoJ4sATWIJQnJMwpY6u3E MWDBQzK5mOpiAHqbaITSWiyx7pnJJwj3DrFTvyXJyceR+BaDA7m/qIQbmesf41jDE9qejUm dBNbVdRhSFwLWFVYFcfQAliqxHaqS0TG3kAwAMIBBsy9E117PAimMtMI/9Cu3ZeUPtrLVmz rvXqnXa6m6RHAToujZUNw== X-UI-Out-Filterresults: notjunk:1;V01:K0:ckeQPOCe0dk=:7uTm1jcwDsedWnp7FbCrJV Tkk1zHQX8nE+p/+F5QKzcL1RPguJNuJhS7t6m6aQHX3CXdqmJzSD7JUfymWtWTZtUkD2WDIAb QYWx+CL2PQ/NcbgiRPhsBQpnYcrc7ggIi1invrOod69LSAeoTEH+jM5YK+gyNFADBIxGZJB+l PD3arokcnc+O62Ok/uljAOGjaElTj15pfJ9YjwjKX+9Jyi4pxwfr3MTzEdGx/pBFEvWsJ21sW B9B5BHIUaErpAs/BDXVAIKG9WsLR9FfBBJWJcJofRvhULxdr84dWy0zGlg0/wO0nDr9lWQWi0 5Mk74+gxQMsoWVj6Yt9PVY1T2Yo7X5g9Pnw5i+hf3ecofTHOdMRocJaNKnKT7SEeX2vVVoreS jHGZDmntqid6E88c7AtOjkWNX5j2zFcWKK3XFyvc892j4rWZNIUxwC+F++bzCgEP+vWjw0CEC x/yzWMBL8xiD4/fxamh53tTT1yR8U+OM95sdh4ynPMooqBDxLHvWK5leMJne2hKsWxAaSP/Ct nDvAD4y4YvY/y1+2xDtzMJ875JoBhK6FfZujFS+pP1qQwks/tqJz3c0wASFlM+XfgYtQIuGSy V9aABdFf7iUCnLE68jt9m3advjLiWRI6I/queLDnZTLhnYbUiH4Vwt0m5eFtOGAtQVrKVxsv6 fNevxSuNP1ux6DCMEdJs/PAuJsBvGlTNAygqM/PnFIo6JW2ToDUJXseBFSb9v2PXAV2AMMl7Q ReJ384fpzHCzh0br Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday 04 February 2016 10:59:31 Andrew Morton wrote: > On Thu, 04 Feb 2016 13:40:38 +0100 Arnd Bergmann wrote: > > > diff --git a/include/linux/err.h b/include/linux/err.h > > index b7d4a9ff6342..bd4936a2c352 100644 > > --- a/include/linux/err.h > > +++ b/include/linux/err.h > > @@ -18,9 +18,7 @@ > > > > #ifndef __ASSEMBLY__ > > > > -#define IS_ERR_VALUE(x) ((typeof(x))(-1) <= 0 \ > > - ? unlikely((x) <= -1) \ > > - : unlikely((x) >= (typeof(x))-MAX_ERRNO)) > > +#define IS_ERR_VALUE(x) (unlikely((unsigned long long)(x) >= (unsigned long long)(typeof(x))-MAX_ERRNO)) > > > > static inline void * __must_check ERR_PTR(long error) > > { > > > > > > I'm not sure if the cast to 'unsigned long long' might cause less > > efficient code to be generated by gcc. I would hope that it is smart > > enough to not actually extend shorter variables to 64 bit before > > doing the comparison but I have not checked yet. > > I did a quick test with i386 on drivers/nvmem/core.o. The patch takes > the text size from 9098 bytes to 9133. That file has 11 instances of > IS_ERR_VALUE(). This seems to be because it brings back the logic to what it was before in case of 'int' arguments. I checked the assembly output and found mine to be identical to v4.4 in this case: text data bss dec hex filename v4.4 9942 1872 2856 14670 394e drivers/nvmem/core.o a.hajda 9922 1872 2856 14650 393a drivers/nvmem/core.o arnd 9942 1872 2856 14670 394e drivers/nvmem/core.o Andrzej's version is a little shorter on ARM because in case of signed numbers it only checks for negative values, rather than checking for values in the [-MAX_ERRNO..-1] range. I think the original behavior is more logical in this case, and my version restores it. Looking at drivers/char/mem.o, which had an actual bug that was fixed by Andrzej's patch, the output with my version and his is identical (failing an lseek on /dev/mem to offset 0xfffffffffffffe00 or higher, instead of failing for offset 0x00000000fffffe00-0x00000000ffffffff). Arnd