| Message ID | 1519301715-31798-6-git-send-email-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Masahiro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc2 next-20180223]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/linux-const-h-cleanups-of-macros-such-as-UL-_BITUL-BIT-etc/20180224-110702
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/const.h:4:0,
from include/linux/bitops.h:4,
from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from drivers/infiniband/hw/bnxt_re/ib_verbs.c:39:
drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function 'bnxt_re_query_device':
>> include/uapi/linux/const.h:28:27: warning: left shift count >= width of type [-Wshift-count-overflow]
#define _BITUL(x) (_UL(1) << (x))
^
>> include/linux/const.h:9:18: note: in expansion of macro '_BITUL'
#define BIT(x) (_BITUL(x))
^~~~~~
drivers/infiniband/hw/bnxt_re/bnxt_re.h:61:34: note: in expansion of macro 'BIT'
#define BNXT_RE_MAX_MR_SIZE_HIGH BIT(39)
^~~
drivers/infiniband/hw/bnxt_re/bnxt_re.h:62:30: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE_HIGH'
#define BNXT_RE_MAX_MR_SIZE BNXT_RE_MAX_MR_SIZE_HIGH
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/infiniband/hw/bnxt_re/ib_verbs.c:149:25: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE'
ib_attr->max_mr_size = BNXT_RE_MAX_MR_SIZE;
^~~~~~~~~~~~~~~~~~~
drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function 'bnxt_re_reg_user_mr':
>> include/uapi/linux/const.h:28:27: warning: left shift count >= width of type [-Wshift-count-overflow]
#define _BITUL(x) (_UL(1) << (x))
^
>> include/linux/const.h:9:18: note: in expansion of macro '_BITUL'
#define BIT(x) (_BITUL(x))
^~~~~~
drivers/infiniband/hw/bnxt_re/bnxt_re.h:61:34: note: in expansion of macro 'BIT'
#define BNXT_RE_MAX_MR_SIZE_HIGH BIT(39)
^~~
drivers/infiniband/hw/bnxt_re/bnxt_re.h:62:30: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE_HIGH'
#define BNXT_RE_MAX_MR_SIZE BNXT_RE_MAX_MR_SIZE_HIGH
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/infiniband/hw/bnxt_re/ib_verbs.c:3588:15: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE'
if (length > BNXT_RE_MAX_MR_SIZE) {
^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/const.h:28:27: warning: left shift count >= width of type [-Wshift-count-overflow]
#define _BITUL(x) (_UL(1) << (x))
^
>> include/linux/const.h:9:18: note: in expansion of macro '_BITUL'
#define BIT(x) (_BITUL(x))
^~~~~~
drivers/infiniband/hw/bnxt_re/bnxt_re.h:61:34: note: in expansion of macro 'BIT'
#define BNXT_RE_MAX_MR_SIZE_HIGH BIT(39)
^~~
drivers/infiniband/hw/bnxt_re/bnxt_re.h:62:30: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE_HIGH'
#define BNXT_RE_MAX_MR_SIZE BNXT_RE_MAX_MR_SIZE_HIGH
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/infiniband/hw/bnxt_re/ib_verbs.c:3590:12: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE'
length, BNXT_RE_MAX_MR_SIZE);
^~~~~~~~~~~~~~~~~~~
vim +28 include/uapi/linux/const.h
5289f87f3 Masahiro Yamada 2018-02-22 27
4fac4e1b2 Masahiro Yamada 2018-02-22 @28 #define _BITUL(x) (_UL(1) << (x))
4fac4e1b2 Masahiro Yamada 2018-02-22 29 #define _BITULL(x) (_ULL(1) << (x))
2fc016c5b H. Peter Anvin 2013-04-27 30
:::::: The code at line 28 was first introduced by commit
:::::: 4fac4e1b26bc6cfec630fb48920c391d99a44940 linux/const.h: refactor _BITUL and _BITULL a bit
:::::: TO: Masahiro Yamada <yamada.masahiro@socionext.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi test robot, 2018-02-24 14:00 GMT+09:00 kbuild test robot <lkp@intel.com>: > Hi Masahiro, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on linus/master] > [also build test WARNING on v4.16-rc2 next-20180223] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/linux-const-h-cleanups-of-macros-such-as-UL-_BITUL-BIT-etc/20180224-110702 > config: i386-allmodconfig (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 I think this is a false positive report because I see the same warning in the mainline. The cause of the problem is #define BNXT_RE_MAX_MR_SIZE_HIGH BIT(39) in drivers/infiniband/hw/bnxt_re/bnxt_re.h This causes -Wshift-count-overflow warning if you compile test it on 32 bit arches. I did not change the BIT() definition, so this is not my fault. > All warnings (new ones prefixed by >>): > > In file included from include/linux/const.h:4:0, > from include/linux/bitops.h:4, > from include/linux/kernel.h:11, > from include/linux/interrupt.h:6, > from drivers/infiniband/hw/bnxt_re/ib_verbs.c:39: > drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function 'bnxt_re_query_device': >>> include/uapi/linux/const.h:28:27: warning: left shift count >= width of type [-Wshift-count-overflow] > #define _BITUL(x) (_UL(1) << (x)) > ^ >>> include/linux/const.h:9:18: note: in expansion of macro '_BITUL' > #define BIT(x) (_BITUL(x)) > ^~~~~~ > drivers/infiniband/hw/bnxt_re/bnxt_re.h:61:34: note: in expansion of macro 'BIT' > #define BNXT_RE_MAX_MR_SIZE_HIGH BIT(39) > ^~~ > drivers/infiniband/hw/bnxt_re/bnxt_re.h:62:30: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE_HIGH' > #define BNXT_RE_MAX_MR_SIZE BNXT_RE_MAX_MR_SIZE_HIGH > ^~~~~~~~~~~~~~~~~~~~~~~~ > drivers/infiniband/hw/bnxt_re/ib_verbs.c:149:25: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE' > ib_attr->max_mr_size = BNXT_RE_MAX_MR_SIZE; > ^~~~~~~~~~~~~~~~~~~ > drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function 'bnxt_re_reg_user_mr': >>> include/uapi/linux/const.h:28:27: warning: left shift count >= width of type [-Wshift-count-overflow] > #define _BITUL(x) (_UL(1) << (x)) > ^ >>> include/linux/const.h:9:18: note: in expansion of macro '_BITUL' > #define BIT(x) (_BITUL(x)) > ^~~~~~ > drivers/infiniband/hw/bnxt_re/bnxt_re.h:61:34: note: in expansion of macro 'BIT' > #define BNXT_RE_MAX_MR_SIZE_HIGH BIT(39) > ^~~ > drivers/infiniband/hw/bnxt_re/bnxt_re.h:62:30: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE_HIGH' > #define BNXT_RE_MAX_MR_SIZE BNXT_RE_MAX_MR_SIZE_HIGH > ^~~~~~~~~~~~~~~~~~~~~~~~ > drivers/infiniband/hw/bnxt_re/ib_verbs.c:3588:15: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE' > if (length > BNXT_RE_MAX_MR_SIZE) { > ^~~~~~~~~~~~~~~~~~~ >>> include/uapi/linux/const.h:28:27: warning: left shift count >= width of type [-Wshift-count-overflow] > #define _BITUL(x) (_UL(1) << (x)) > ^ >>> include/linux/const.h:9:18: note: in expansion of macro '_BITUL' > #define BIT(x) (_BITUL(x)) > ^~~~~~ > drivers/infiniband/hw/bnxt_re/bnxt_re.h:61:34: note: in expansion of macro 'BIT' > #define BNXT_RE_MAX_MR_SIZE_HIGH BIT(39) > ^~~ > drivers/infiniband/hw/bnxt_re/bnxt_re.h:62:30: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE_HIGH' > #define BNXT_RE_MAX_MR_SIZE BNXT_RE_MAX_MR_SIZE_HIGH > ^~~~~~~~~~~~~~~~~~~~~~~~ > drivers/infiniband/hw/bnxt_re/ib_verbs.c:3590:12: note: in expansion of macro 'BNXT_RE_MAX_MR_SIZE' > length, BNXT_RE_MAX_MR_SIZE); > ^~~~~~~~~~~~~~~~~~~ > > vim +28 include/uapi/linux/const.h > > 5289f87f3 Masahiro Yamada 2018-02-22 27 > 4fac4e1b2 Masahiro Yamada 2018-02-22 @28 #define _BITUL(x) (_UL(1) << (x)) > 4fac4e1b2 Masahiro Yamada 2018-02-22 29 #define _BITULL(x) (_ULL(1) << (x)) > 2fc016c5b H. Peter Anvin 2013-04-27 30 > > :::::: The code at line 28 was first introduced by commit > :::::: 4fac4e1b26bc6cfec630fb48920c391d99a44940 linux/const.h: refactor _BITUL and _BITULL a bit > > :::::: TO: Masahiro Yamada <yamada.masahiro@socionext.com> > :::::: CC: 0day robot <fengguang.wu@intel.com> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Andrew, 2018-02-22 21:15 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>: > Commit 2fc016c5bd8a ("linux/const.h: Add _BITUL() and _BITULL()") > introduced _BITUL() and _BITULL(). Its git-log says the difference > from the already existing BIT() are: > > 1. The namespace is such that they can be used in uapi definitions. > 2. The type is set with the _AC() macro to allow it to be used in > assembly. > 3. The type is explicitly specified to be UL or ULL. > > However, I found _BITUL() is mostly used in kernel-space since it is > handy to share headers between C and assembly. If we only need '2.', > we can improve the existing BIT() for use in assembly, allowing us to > avoid unnecessary underscore prefixes. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- This patch conflicts with the one from Will Deacon. (https://patchwork.kernel.org/patch/10242531/) I think his patch is better than mine. So, could you drop this one from your tree, please? (only this one from the series.) Thanks > V2: https://patchwork.kernel.org/patch/9498271/ > > Changes in v3: None > Changes in v2: None > > include/linux/bitops.h | 3 +-- > include/linux/const.h | 3 +++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index 4cac4e1..8a856be 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -1,11 +1,10 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > #ifndef _LINUX_BITOPS_H > #define _LINUX_BITOPS_H > +#include <linux/const.h> > #include <asm/types.h> > > #ifdef __KERNEL__ > -#define BIT(nr) (1UL << (nr)) > -#define BIT_ULL(nr) (1ULL << (nr)) > #define BIT_MASK(nr) (1UL << ((nr) % BITS_PER_LONG)) > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > #define BIT_ULL_MASK(nr) (1ULL << ((nr) % BITS_PER_LONG_LONG)) > diff --git a/include/linux/const.h b/include/linux/const.h > index 7b55a55..200892d 100644 > --- a/include/linux/const.h > +++ b/include/linux/const.h > @@ -6,4 +6,7 @@ > #define UL(x) (_UL(x)) > #define ULL(x) (_ULL(x)) > > +#define BIT(x) (_BITUL(x)) > +#define BIT_ULL(x) (_BITULL(x)) > + > #endif /* _LINUX_CONST_H */ > -- > 2.7.4 >
diff --git a/include/linux/bitops.h b/include/linux/bitops.h index 4cac4e1..8a856be 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -1,11 +1,10 @@ /* SPDX-License-Identifier: GPL-2.0 */ #ifndef _LINUX_BITOPS_H #define _LINUX_BITOPS_H +#include <linux/const.h> #include <asm/types.h> #ifdef __KERNEL__ -#define BIT(nr) (1UL << (nr)) -#define BIT_ULL(nr) (1ULL << (nr)) #define BIT_MASK(nr) (1UL << ((nr) % BITS_PER_LONG)) #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) #define BIT_ULL_MASK(nr) (1ULL << ((nr) % BITS_PER_LONG_LONG)) diff --git a/include/linux/const.h b/include/linux/const.h index 7b55a55..200892d 100644 --- a/include/linux/const.h +++ b/include/linux/const.h @@ -6,4 +6,7 @@ #define UL(x) (_UL(x)) #define ULL(x) (_ULL(x)) +#define BIT(x) (_BITUL(x)) +#define BIT_ULL(x) (_BITULL(x)) + #endif /* _LINUX_CONST_H */
Commit 2fc016c5bd8a ("linux/const.h: Add _BITUL() and _BITULL()") introduced _BITUL() and _BITULL(). Its git-log says the difference from the already existing BIT() are: 1. The namespace is such that they can be used in uapi definitions. 2. The type is set with the _AC() macro to allow it to be used in assembly. 3. The type is explicitly specified to be UL or ULL. However, I found _BITUL() is mostly used in kernel-space since it is handy to share headers between C and assembly. If we only need '2.', we can improve the existing BIT() for use in assembly, allowing us to avoid unnecessary underscore prefixes. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- V2: https://patchwork.kernel.org/patch/9498271/ Changes in v3: None Changes in v2: None include/linux/bitops.h | 3 +-- include/linux/const.h | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-)