[v3,5/5] linux/const.h: move BIT(_ULL) to linux/const.h for use in assembly
diff mbox

Message ID 1519301715-31798-6-git-send-email-yamada.masahiro@socionext.com
State New, archived
Headers show

Commit Message

Masahiro Yamada Feb. 22, 2018, 12:15 p.m. UTC
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(-)

Comments

kernel test robot Feb. 24, 2018, 5 a.m. UTC | #1
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
Masahiro Yamada Feb. 26, 2018, 2:13 a.m. UTC | #2
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
Masahiro Yamada March 13, 2018, 8:39 a.m. UTC | #3
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
>

Patch
diff mbox

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 */