diff mbox series

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

Message ID 1519301715-31798-6-git-send-email-yamada.masahiro@socionext.com
State New
Headers show
Series linux/const.h: cleanups of macros such as UL(), _BITUL(), BIT() etc. | expand

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(-)

-- 
2.7.4

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




-- 
Best Regards
Masahiro Yamada
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

>




-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

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