[PATCH v3 0/5] linux/const.h: cleanups of macros such as UL(), _BITUL(), BIT() etc.

[PATCH v3 0/5] linux/const.h: cleanups of macros such as UL(), _BITUL(), BIT() etc.

[Masahiro Yamada]

ARM, ARM64, UniCore32 define UL() as a shorthand of _AC(..., UL).
More architectures may introduce it in the future.

UL() is arch-agnostic, and useful. So let's move it to
include/linux/const.h

Currently, <asm/memory.h> must be included to use UL().
It pulls in more bloats just for defining some bit macros.

I posted V2 one year ago.

The previous posts are:
https://patchwork.kernel.org/patch/9498273/
https://patchwork.kernel.org/patch/9498275/
https://patchwork.kernel.org/patch/9498269/
https://patchwork.kernel.org/patch/9498271/

At that time, what blocked this series was a comment from
David Howells:
  You need to be very careful doing this.  Some userspace stuff
  depends on the guard macro names on the kernel header files.

(https://patchwork.kernel.org/patch/9498275/)

Looking at the code closer, I noticed this is not a problem.

See the following line.
https://github.com/torvalds/linux/blob/v4.16-rc2/scripts/headers_install.sh#L40

scripts/headers_install.sh rips off _UAPI prefix from guard macro names.

I ran "make headers_install" and confirmed the result is what I expect.

So, we can prefix the include guard of include/uapi/linux/const.h,
and add a new include/linux/const.h.



Masahiro Yamada (5):
  linux/const.h: prefix include guard of uapi/linux/const.h with _UAPI
  m68k: rename UL() to TO_UL()
  linux/const.h: move UL() macro to include/linux/const.h
  linux/const.h: refactor _BITUL and _BITULL a bit
  linux/const.h: move BIT(_ULL) to linux/const.h for use in assembly

 arch/arm/include/asm/memory.h       |  6 ------
 arch/arm64/include/asm/memory.h     |  6 ------
 arch/m68k/mm/init.c                 |  6 +++---
 arch/unicore32/include/asm/memory.h |  6 ------
 include/linux/bitops.h              |  3 +--
 include/linux/const.h               | 12 ++++++++++++
 include/uapi/linux/const.h          | 13 ++++++++-----
 7 files changed, 24 insertions(+), 28 deletions(-)
 create mode 100644 include/linux/const.h

-- 
2.7.4

[PATCH v3 1/5] linux/const.h: prefix include guard of uapi/linux/const.h with _UAPI

[Masahiro Yamada]

I am going to add include/linux/const.h for the kernel space.

Add _UAPI to the include guard of include/uapi/linux/const.h to
prepare for that.

Please notice the guard name of the exported one will be kept as-is.
So, this commit has no impact to the userspace even if some userspace
stuff depends on the guard macro names.

scripts/headers_install.sh processes exported headers by SED, and
rips off "_UAPI" from guard macro names.

  #ifndef _UAPI_LINUX_CONST_H
  #define _UAPI_LINUX_CONST_H

will be turned into

  #ifndef _LINUX_CONST_H
  #define _LINUX_CONST_H

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v3:
  - Split out as a prerequisite patch and describe detailed information

Changes in v2: None

 include/uapi/linux/const.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
index 9253775..c5a60eb 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -1,8 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
 /* const.h: Macros for dealing with constants.  */
 
-#ifndef _LINUX_CONST_H
-#define _LINUX_CONST_H
+#ifndef _UAPI_LINUX_CONST_H
+#define _UAPI_LINUX_CONST_H
 
 /* Some constant macros are used in both assembler and
  * C code.  Therefore we cannot annotate them always with
@@ -25,4 +25,4 @@
 #define _BITUL(x)	(_AC(1,UL) << (x))
 #define _BITULL(x)	(_AC(1,ULL) << (x))
 
-#endif /* !(_LINUX_CONST_H) */
+#endif /* _UAPI_LINUX_CONST_H */
-- 
2.7.4

[PATCH v3 2/5] m68k: rename UL() to TO_UL()

[Masahiro Yamada]

ARM, ARM64 and UniCore32 define UL(x) like follows:
  #define UL(x) _AC(x, UL)

While, M68K defines it differently:
  #define UL(x) ((unsigned long) (x))

I want to move the former to a common header.  Beforehand, this
commit renames the latter to avoid name conflict.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
V2: https://patchwork.kernel.org/patch/9498273/

Changes in v3: None
Changes in v2:
 - Split out as a prerequisite patch

 arch/m68k/mm/init.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/m68k/mm/init.c b/arch/m68k/mm/init.c
index e85acd1..583a8e5 100644
--- a/arch/m68k/mm/init.c
+++ b/arch/m68k/mm/init.c
@@ -122,9 +122,9 @@ void free_initmem(void)
 
 void __init print_memmap(void)
 {
-#define UL(x) ((unsigned long) (x))
-#define MLK(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 10
-#define MLM(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 20
+#define TO_UL(x) ((unsigned long) (x))
+#define MLK(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 10
+#define MLM(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 20
 #define MLK_ROUNDUP(b, t) b, t, DIV_ROUND_UP(((t) - (b)), 1024)
 
 	pr_notice("Virtual kernel memory layout:\n"
-- 
2.7.4

[PATCH v3 3/5] linux/const.h: move UL() macro to include/linux/const.h

[Masahiro Yamada]

ARM, ARM64 and UniCore32 duplicate the definition of UL():

  #define UL(x) _AC(x, UL)

This is not actually arch-specific, so it will be useful to move it
to a common header.  Currently, we only have the uapi variant for
linux/const.h, so I am creating include/linux/const.h.

I also added _UL(), _ULL() and ULL() because _AC() is mostly used in
the form either _AC(..., UL) or _AC(..., ULL).  I expect they will be
replaced in follow-up cleanups.  The underscore-prefixed ones should
be used for exported headers.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
V2: https://patchwork.kernel.org/patch/9498275/


Changes in v3: None
Changes in v2: None

 arch/arm/include/asm/memory.h       | 6 ------
 arch/arm64/include/asm/memory.h     | 6 ------
 arch/unicore32/include/asm/memory.h | 6 ------
 include/linux/const.h               | 9 +++++++++
 include/uapi/linux/const.h          | 3 +++
 5 files changed, 12 insertions(+), 18 deletions(-)
 create mode 100644 include/linux/const.h

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 4966677..ed8fd0d 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -22,12 +22,6 @@
 #include <mach/memory.h>
 #endif
 
-/*
- * Allow for constants defined here to be used from assembly code
- * by prepending the UL suffix only with actual C code compilation.
- */
-#define UL(x) _AC(x, UL)
-
 /* PAGE_OFFSET - the virtual address of the start of the kernel image */
 #define PAGE_OFFSET		UL(CONFIG_PAGE_OFFSET)
 
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 50fa96a..49d9921 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -29,12 +29,6 @@
 #include <asm/sizes.h>
 
 /*
- * Allow for constants defined here to be used from assembly code
- * by prepending the UL suffix only with actual C code compilation.
- */
-#define UL(x) _AC(x, UL)
-
-/*
  * Size of the PCI I/O space. This must remain a power of two so that
  * IO_SPACE_LIMIT acts as a mask for the low bits of I/O addresses.
  */
diff --git a/arch/unicore32/include/asm/memory.h b/arch/unicore32/include/asm/memory.h
index 3bb0a29..66bb9f6 100644
--- a/arch/unicore32/include/asm/memory.h
+++ b/arch/unicore32/include/asm/memory.h
@@ -20,12 +20,6 @@
 #include <mach/memory.h>
 
 /*
- * Allow for constants defined here to be used from assembly code
- * by prepending the UL suffix only with actual C code compilation.
- */
-#define UL(x) _AC(x, UL)
-
-/*
  * PAGE_OFFSET - the virtual address of the start of the kernel image
  * TASK_SIZE - the maximum size of a user space task.
  * TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area
diff --git a/include/linux/const.h b/include/linux/const.h
new file mode 100644
index 0000000..7b55a55
--- /dev/null
+++ b/include/linux/const.h
@@ -0,0 +1,9 @@
+#ifndef _LINUX_CONST_H
+#define _LINUX_CONST_H
+
+#include <uapi/linux/const.h>
+
+#define UL(x)		(_UL(x))
+#define ULL(x)		(_ULL(x))
+
+#endif /* _LINUX_CONST_H */
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
index c5a60eb..09bc0e0 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -22,6 +22,9 @@
 #define _AT(T,X)	((T)(X))
 #endif
 
+#define _UL(x)		(_AC(x, UL))
+#define _ULL(x)		(_AC(x, ULL))
+
 #define _BITUL(x)	(_AC(1,UL) << (x))
 #define _BITULL(x)	(_AC(1,ULL) << (x))
 
-- 
2.7.4

[PATCH v3 4/5] linux/const.h: refactor _BITUL and _BITULL a bit

[Masahiro Yamada]

Minor cleanups available by _UL and _ULL.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
V2: https://patchwork.kernel.org/patch/9498269/


Changes in v3: None
Changes in v2: None

 include/uapi/linux/const.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
index 09bc0e0..5ed721a 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -25,7 +25,7 @@
 #define _UL(x)		(_AC(x, UL))
 #define _ULL(x)		(_AC(x, ULL))
 
-#define _BITUL(x)	(_AC(1,UL) << (x))
-#define _BITULL(x)	(_AC(1,ULL) << (x))
+#define _BITUL(x)	(_UL(1) << (x))
+#define _BITULL(x)	(_ULL(1) << (x))
 
 #endif /* _UAPI_LINUX_CONST_H */
-- 
2.7.4

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

[Masahiro Yamada]

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

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

Re: [PATCH v3 2/5] m68k: rename UL() to TO_UL()

[Geert Uytterhoeven]

Hi Yamada-san,

On Thu, Feb 22, 2018 at 1:15 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> ARM, ARM64 and UniCore32 define UL(x) like follows:
>   #define UL(x) _AC(x, UL)
>
> While, M68K defines it differently:
>   #define UL(x) ((unsigned long) (x))
>
> I want to move the former to a common header.  Beforehand, this
> commit renames the latter to avoid name conflict.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> V2: https://patchwork.kernel.org/patch/9498273/
>
> Changes in v3: None
> Changes in v2:
>  - Split out as a prerequisite patch
>
>  arch/m68k/mm/init.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/m68k/mm/init.c b/arch/m68k/mm/init.c
> index e85acd1..583a8e5 100644
> --- a/arch/m68k/mm/init.c
> +++ b/arch/m68k/mm/init.c
> @@ -122,9 +122,9 @@ void free_initmem(void)
>
>  void __init print_memmap(void)
>  {
> -#define UL(x) ((unsigned long) (x))
> -#define MLK(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 10
> -#define MLM(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 20
> +#define TO_UL(x) ((unsigned long) (x))
> +#define MLK(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 10
> +#define MLM(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 20
>  #define MLK_ROUNDUP(b, t) b, t, DIV_ROUND_UP(((t) - (b)), 1024)

Please note that this code patch is scheduled for removal in v4.17, cfr.
"[PATCH] m68k/mm: Stop printing the virtual memory layout"
(https://lkml.org/lkml/2018/2/12/97).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 2/5] m68k: rename UL() to TO_UL()

[Masahiro Yamada]

Hi Geert

2018-02-22 22:20 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> Hi Yamada-san,
>
> On Thu, Feb 22, 2018 at 1:15 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> ARM, ARM64 and UniCore32 define UL(x) like follows:
>>   #define UL(x) _AC(x, UL)
>>
>> While, M68K defines it differently:
>>   #define UL(x) ((unsigned long) (x))
>>
>> I want to move the former to a common header.  Beforehand, this
>> commit renames the latter to avoid name conflict.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> V2: https://patchwork.kernel.org/patch/9498273/
>>
>> Changes in v3: None
>> Changes in v2:
>>  - Split out as a prerequisite patch
>>
>>  arch/m68k/mm/init.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/m68k/mm/init.c b/arch/m68k/mm/init.c
>> index e85acd1..583a8e5 100644
>> --- a/arch/m68k/mm/init.c
>> +++ b/arch/m68k/mm/init.c
>> @@ -122,9 +122,9 @@ void free_initmem(void)
>>
>>  void __init print_memmap(void)
>>  {
>> -#define UL(x) ((unsigned long) (x))
>> -#define MLK(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 10
>> -#define MLM(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 20
>> +#define TO_UL(x) ((unsigned long) (x))
>> +#define MLK(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 10
>> +#define MLM(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 20
>>  #define MLK_ROUNDUP(b, t) b, t, DIV_ROUND_UP(((t) - (b)), 1024)
>
> Please note that this code patch is scheduled for removal in v4.17, cfr.
> "[PATCH] m68k/mm: Stop printing the virtual memory layout"
> (https://lkml.org/lkml/2018/2/12/97).
>


I see, but I do not see it in linux-next as of writing.


Without this prerequisite, 3/5 would cause a build error.
So, I needed to include it in this series.

I am hoping this series will be picked up by Andrew Morton.
In my understanding, he applies patches on top of the linux-next.


I think either will happen:

[1] If your patch appears in linux-next first,
    my 2/5 will be skipped, and the rest of the series will be applied.

[2] If my series is applied first,
    Andrew will drop 2/5 when your patch appears in linux-next
        (this is simply detected by patch conflict)


Andrew, please correct me if I am wrong.


Thanks!


-- 
Best Regards
Masahiro Yamada

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4398 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63097 bytes --]

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

[Masahiro Yamada]

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

Re: [PATCH v3 2/5] m68k: rename UL() to TO_UL()

[Geert Uytterhoeven]

Hi Yamada-san,

CC sfr (conflict heads up)

On Thu, Feb 22, 2018 at 5:58 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2018-02-22 22:20 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>> On Thu, Feb 22, 2018 at 1:15 PM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> ARM, ARM64 and UniCore32 define UL(x) like follows:
>>>   #define UL(x) _AC(x, UL)
>>>
>>> While, M68K defines it differently:
>>>   #define UL(x) ((unsigned long) (x))
>>>
>>> I want to move the former to a common header.  Beforehand, this
>>> commit renames the latter to avoid name conflict.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>> V2: https://patchwork.kernel.org/patch/9498273/
>>>
>>> Changes in v3: None
>>> Changes in v2:
>>>  - Split out as a prerequisite patch
>>>
>>>  arch/m68k/mm/init.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/m68k/mm/init.c b/arch/m68k/mm/init.c
>>> index e85acd1..583a8e5 100644
>>> --- a/arch/m68k/mm/init.c
>>> +++ b/arch/m68k/mm/init.c
>>> @@ -122,9 +122,9 @@ void free_initmem(void)
>>>
>>>  void __init print_memmap(void)
>>>  {
>>> -#define UL(x) ((unsigned long) (x))
>>> -#define MLK(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 10
>>> -#define MLM(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 20
>>> +#define TO_UL(x) ((unsigned long) (x))
>>> +#define MLK(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 10
>>> +#define MLM(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 20
>>>  #define MLK_ROUNDUP(b, t) b, t, DIV_ROUND_UP(((t) - (b)), 1024)
>>
>> Please note that this code patch is scheduled for removal in v4.17, cfr.
>> "[PATCH] m68k/mm: Stop printing the virtual memory layout"
>> (https://lkml.org/lkml/2018/2/12/97).
>
> I see, but I do not see it in linux-next as of writing.

It will be tomorrow.

> Without this prerequisite, 3/5 would cause a build error.
> So, I needed to include it in this series.
>
> I am hoping this series will be picked up by Andrew Morton.
> In my understanding, he applies patches on top of the linux-next.
>
> I think either will happen:
>
> [1] If your patch appears in linux-next first,
>     my 2/5 will be skipped, and the rest of the series will be applied.
>
> [2] If my series is applied first,
>     Andrew will drop 2/5 when your patch appears in linux-next
>         (this is simply detected by patch conflict)

OK.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Masahiro Yamada]

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