Skip to contentSkip to conversation

memblock: fix section mismatch warning

PATCH Applied Arnd Bergmann 3 participants 7 messages Feb 25, 2021

Subsystems (2)
Memory management memblock and memory management initialization
lore
Share & tools
lore

Arnd Bergmann

author
#0 lore raw

PATCH 1/1 memblock: fix section mismatch warning

From: Arnd Bergmann <[email protected]>

The inlining logic in clang-13 is rewritten to often not inline some functions that were inlined by all earlier compilers.

In case of the memblock interfaces, this exposed a harmless bug of a missing __init annotation:

WARNING: modpost: vmlinux.o(.text+0x507c0a): Section mismatch in reference from the function memblock_bottom_up() to the variable .meminit.data:memblock The function memblock_bottom_up() references
the variable __meminitdata memblock.
This is often because memblock_bottom_up lacks a __meminitdata
annotation or the annotation of memblock is wrong.

Interestingly, these annotations were present originally, but got removed with the explanation that the __init annotation prevents the function from getting inlined. I checked this again and found that while this is the case with clang, gcc (version 7 through 10, did not test others) does inline the functions regardless.

As the previous change was apparently intended to help the clang builds, reverting it to help the newer clang versions seems appropriate as well. gcc builds don't seem to care either way.

Fixes 5bdba520c1b3 ("mm: memblock: drop __init from memblock functions to make it inline")

Reference: 2cfb3665e864 ("include/linux/memblock.h: add __init to memblock_set_bottom_up()")

Signed-off-by Arnd Bergmann <[email protected]>
---
 include/linux/memblock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/memblock.h +2 -2
index c88bc24e31aa..d13e3cd938b4 100644
@@ -460,7 +460,7 @@ static inline void memblock_free_late(phys_addr_t base, phys_addr_t size)
 /*
  * Set the allocation direction to bottom-up or top-down.
  */
-static inline void memblock_set_bottom_up(bool enable)
+static inline __init void memblock_set_bottom_up(bool enable)
 {
 	memblock.bottom_up = enable;
 }

@@ -470,7 +470,7 @@ static inline void memblock_set_bottom_up(bool enable)
  * if this is true, that said, memblock will allocate memory
  * in bottom-up direction.
  */
-static inline bool memblock_bottom_up(void)
+static inline __init bool memblock_bottom_up(void)
 {
 	return memblock.bottom_up;
 }
Signature · 1 line
2.29.2
#1 lore raw
Quoted 14 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The inlining logic in clang-13 is rewritten to often not inline some functions that were inlined by all earlier compilers.

In case of the memblock interfaces, this exposed a harmless bug of a missing __init annotation:

WARNING: modpost: vmlinux.o(.text+0x507c0a): Section mismatch in reference from the function memblock_bottom_up() to the variable .meminit.data:memblock The function memblock_bottom_up() references
the variable __meminitdata memblock.
This is often because memblock_bottom_up lacks a __meminitdata
annotation or the annotation of memblock is wrong.

Quoted 5 lines

Interestingly, these annotations were present originally, but got removed with the explanation that the __init annotation prevents the function from getting inlined. I checked this again and found that while this is the case with clang, gcc (version 7 through 10, did not test others) does inline the functions regardless.

Did I understand correctly, that with this change it will not get inlined with any version of clang? Maybe __always_inline is more appropriate then.

(I don't see why to not inline that function, but I am obviously not a compiler person :) )

Signature · 3 lines
Thanks,

David / dhildenb
author
#2 lore raw
Quoted 22 lines · David Hildenbrand
Quoted 14 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The inlining logic in clang-13 is rewritten to often not inline some functions that were inlined by all earlier compilers.

In case of the memblock interfaces, this exposed a harmless bug of a missing __init annotation:

WARNING: modpost: vmlinux.o(.text+0x507c0a): Section mismatch in reference from the function memblock_bottom_up() to the variable .meminit.data:memblock The function memblock_bottom_up() references
the variable __meminitdata memblock.
This is often because memblock_bottom_up lacks a __meminitdata
annotation or the annotation of memblock is wrong.

Quoted 5 lines

Interestingly, these annotations were present originally, but got removed with the explanation that the __init annotation prevents the function from getting inlined. I checked this again and found that while this is the case with clang, gcc (version 7 through 10, did not test others) does inline the functions regardless.

Quoted 6 lines

Did I understand correctly, that with this change it will not get inlined with any version of clang? Maybe __always_inline is more appropriate then.

(I don't see why to not inline that function, but I am obviously not a compiler person :) )

Looking at the assembler output in the arm64 build that triggered the warning, I see this code:

0000000000000a40 <memblock_bottom_up>:

 a40:   55                      push   %rbp
 a41:   48 89 e5                mov    %rsp,%rbp
 a44:   41 56                   push   %r14
 a46:   53                      push   %rbx
 a47:   e8 00 00 00 00          call   a4c <memblock_bottom_up+0xc>
                        a48: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
 a4c:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                        a4f: R_X86_64_32S       memblock
 a53:   e8 00 00 00 00          call   a58 <memblock_bottom_up+0x18>
                        a54: R_X86_64_PLT32     __asan_load1_noabort-0x4
 a58:   44 0f b6 35 00 00 00    movzbl 0x0(%rip),%r14d        # a60
<memblock_bottom_up+0x20>
 a5f:   00
                        a5c: R_X86_64_PC32      memblock-0x4
 a60:   bf 02 00 00 00          mov    $0x2,%edi
 a65:   44 89 f6                mov    %r14d,%esi
 a68:   e8 00 00 00 00          call   a6d <memblock_bottom_up+0x2d>
                        a69: R_X86_64_PLT32
__sanitizer_cov_trace_const_cmp1-0x4
 a6d:   41 83 fe 01             cmp    $0x1,%r14d
 a71:   77 20                   ja     a93 <memblock_bottom_up+0x53>
 a73:   e8 00 00 00 00          call   a78 <memblock_bottom_up+0x38>
                        a74: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
 a78:   44 89 f3                mov    %r14d,%ebx
 a7b:   80 e3 01                and    $0x1,%bl
 a7e:   41 83 e6 01             and    $0x1,%r14d
 a82:   31 ff                   xor    %edi,%edi
 a84:   44 89 f6                mov    %r14d,%esi
 a87:   e8 00 00 00 00          call   a8c <memblock_bottom_up+0x4c>
                        a88: R_X86_64_PLT32
__sanitizer_cov_trace_const_cmp1-0x4
 a8c:   89 d8                   mov    %ebx,%eax
 a8e:   5b                      pop    %rbx
 a8f:   41 5e                   pop    %r14
 a91:   5d                      pop    %rbp
 a92:   c3                      ret
 a93:   e8 00 00 00 00          call   a98 <memblock_bottom_up+0x58>
                        a94: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
 a98:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                        a9b: R_X86_64_32S       .data+0x3c0
 a9f:   4c 89 f6                mov    %r14,%rsi
 aa2:   e8 00 00 00 00          call   aa7 <memblock_bottom_up+0x67>
                        aa3: R_X86_64_PLT32
__ubsan_handle_load_invalid_value-0x4
 aa7:   eb cf                   jmp    a78 <memblock_bottom_up+0x38>
 aa9:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
 ab0:   00 00 00
 ab3:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
 aba:   00 00 00
 abd:   0f 1f 00                nopl   (%rax)

This means that the sanitiers added a lot of extra checking around what would have been a trivial global variable access otherwise. In this case, not inlining would be a reasonable decision.

Arnd

Reviewed-by
#3 lore raw
Quoted 85 lines · Arnd Bergmann
Quoted 22 lines · David Hildenbrand
Quoted 14 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The inlining logic in clang-13 is rewritten to often not inline some functions that were inlined by all earlier compilers.

In case of the memblock interfaces, this exposed a harmless bug of a missing __init annotation:

WARNING: modpost: vmlinux.o(.text+0x507c0a): Section mismatch in reference from the function memblock_bottom_up() to the variable .meminit.data:memblock The function memblock_bottom_up() references
the variable __meminitdata memblock.
This is often because memblock_bottom_up lacks a __meminitdata
annotation or the annotation of memblock is wrong.

Quoted 5 lines

Interestingly, these annotations were present originally, but got removed with the explanation that the __init annotation prevents the function from getting inlined. I checked this again and found that while this is the case with clang, gcc (version 7 through 10, did not test others) does inline the functions regardless.

Quoted 6 lines

Did I understand correctly, that with this change it will not get inlined with any version of clang? Maybe __always_inline is more appropriate then.

(I don't see why to not inline that function, but I am obviously not a compiler person :) )

Looking at the assembler output in the arm64 build that triggered the warning, I see this code:

0000000000000a40 <memblock_bottom_up>:

  a40:   55                      push   %rbp
  a41:   48 89 e5                mov    %rsp,%rbp
  a44:   41 56                   push   %r14
  a46:   53                      push   %rbx
  a47:   e8 00 00 00 00          call   a4c <memblock_bottom_up+0xc>
                         a48: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
  a4c:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                         a4f: R_X86_64_32S       memblock
  a53:   e8 00 00 00 00          call   a58 <memblock_bottom_up+0x18>
                         a54: R_X86_64_PLT32     __asan_load1_noabort-0x4
  a58:   44 0f b6 35 00 00 00    movzbl 0x0(%rip),%r14d        # a60
<memblock_bottom_up+0x20>
  a5f:   00
                         a5c: R_X86_64_PC32      memblock-0x4
  a60:   bf 02 00 00 00          mov    $0x2,%edi
  a65:   44 89 f6                mov    %r14d,%esi
  a68:   e8 00 00 00 00          call   a6d <memblock_bottom_up+0x2d>
                         a69: R_X86_64_PLT32
__sanitizer_cov_trace_const_cmp1-0x4
  a6d:   41 83 fe 01             cmp    $0x1,%r14d
  a71:   77 20                   ja     a93 <memblock_bottom_up+0x53>
  a73:   e8 00 00 00 00          call   a78 <memblock_bottom_up+0x38>
                         a74: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
  a78:   44 89 f3                mov    %r14d,%ebx
  a7b:   80 e3 01                and    $0x1,%bl
  a7e:   41 83 e6 01             and    $0x1,%r14d
  a82:   31 ff                   xor    %edi,%edi
  a84:   44 89 f6                mov    %r14d,%esi
  a87:   e8 00 00 00 00          call   a8c <memblock_bottom_up+0x4c>
                         a88: R_X86_64_PLT32
__sanitizer_cov_trace_const_cmp1-0x4
  a8c:   89 d8                   mov    %ebx,%eax
  a8e:   5b                      pop    %rbx
  a8f:   41 5e                   pop    %r14
  a91:   5d                      pop    %rbp
  a92:   c3                      ret
  a93:   e8 00 00 00 00          call   a98 <memblock_bottom_up+0x58>
                         a94: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
  a98:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                         a9b: R_X86_64_32S       .data+0x3c0
  a9f:   4c 89 f6                mov    %r14,%rsi
  aa2:   e8 00 00 00 00          call   aa7 <memblock_bottom_up+0x67>
                         aa3: R_X86_64_PLT32
__ubsan_handle_load_invalid_value-0x4
  aa7:   eb cf                   jmp    a78 <memblock_bottom_up+0x38>
  aa9:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
  ab0:   00 00 00
  ab3:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
  aba:   00 00 00
  abd:   0f 1f 00                nopl   (%rax)
Quoted 3 lines

This means that the sanitiers added a lot of extra checking around what would have been a trivial global variable access otherwise. In this case, not inlining would be a reasonable decision.

It's not like if there are a lot of call sites:

 $ git grep memblock_bottom_up
arch/x86/mm/init.c:     if (memblock_bottom_up()) {
include/linux/memblock.h:static inline bool memblock_bottom_up(void)
mm/cma.c:               if (!memblock_bottom_up() && memblock_end >= SZ_4G + size) {
mm/memblock.c:  if (memblock_bottom_up())

Similarly for memblock_set_bottom_up() within a kernel image.

But it's not like this is performance-sensitive code :)

Thanks!

Reviewed-by David Hildenbrand <[email protected]>
Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#4 lore raw
In reply to Arnd Bergmann
Quoted 27 lines · Arnd Bergmann
Quoted 26 lines · David Hildenbrand
Quoted 14 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The inlining logic in clang-13 is rewritten to often not inline some functions that were inlined by all earlier compilers.

In case of the memblock interfaces, this exposed a harmless bug of a missing __init annotation:

WARNING: modpost: vmlinux.o(.text+0x507c0a): Section mismatch in reference from the function memblock_bottom_up() to the variable .meminit.data:memblock The function memblock_bottom_up() references
the variable __meminitdata memblock.
This is often because memblock_bottom_up lacks a __meminitdata
annotation or the annotation of memblock is wrong.

Quoted 5 lines

Interestingly, these annotations were present originally, but got removed with the explanation that the __init annotation prevents the function from getting inlined. I checked this again and found that while this is the case with clang, gcc (version 7 through 10, did not test others) does inline the functions regardless.

Did I understand correctly, that with this change it will not get inlined with any version of clang? Maybe __always_inline is more appropriate then.

Quoted 5 lines
Quoted 2 lines

(I don't see why to not inline that function, but I am obviously not a compiler person :) )

Looking at the assembler output in the arm64 build that triggered the warning, I see this code:

"push %rbp" seems more x86 for me, but that's not really important :)

I wonder what happens with other memblock inline APIs, particularly with alloc wrappers. Do they still get inlined?

Quoted 57 lines

0000000000000a40 <memblock_bottom_up>:

 a40:   55                      push   %rbp
 a41:   48 89 e5                mov    %rsp,%rbp
 a44:   41 56                   push   %r14
 a46:   53                      push   %rbx
 a47:   e8 00 00 00 00          call   a4c <memblock_bottom_up+0xc>
                        a48: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
 a4c:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                        a4f: R_X86_64_32S       memblock
 a53:   e8 00 00 00 00          call   a58 <memblock_bottom_up+0x18>
                        a54: R_X86_64_PLT32     __asan_load1_noabort-0x4
 a58:   44 0f b6 35 00 00 00    movzbl 0x0(%rip),%r14d        # a60
<memblock_bottom_up+0x20>
 a5f:   00
                        a5c: R_X86_64_PC32      memblock-0x4
 a60:   bf 02 00 00 00          mov    $0x2,%edi
 a65:   44 89 f6                mov    %r14d,%esi
 a68:   e8 00 00 00 00          call   a6d <memblock_bottom_up+0x2d>
                        a69: R_X86_64_PLT32
__sanitizer_cov_trace_const_cmp1-0x4
 a6d:   41 83 fe 01             cmp    $0x1,%r14d
 a71:   77 20                   ja     a93 <memblock_bottom_up+0x53>
 a73:   e8 00 00 00 00          call   a78 <memblock_bottom_up+0x38>
                        a74: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
 a78:   44 89 f3                mov    %r14d,%ebx
 a7b:   80 e3 01                and    $0x1,%bl
 a7e:   41 83 e6 01             and    $0x1,%r14d
 a82:   31 ff                   xor    %edi,%edi
 a84:   44 89 f6                mov    %r14d,%esi
 a87:   e8 00 00 00 00          call   a8c <memblock_bottom_up+0x4c>
                        a88: R_X86_64_PLT32
__sanitizer_cov_trace_const_cmp1-0x4
 a8c:   89 d8                   mov    %ebx,%eax
 a8e:   5b                      pop    %rbx
 a8f:   41 5e                   pop    %r14
 a91:   5d                      pop    %rbp
 a92:   c3                      ret
 a93:   e8 00 00 00 00          call   a98 <memblock_bottom_up+0x58>
                        a94: R_X86_64_PLT32     __sanitizer_cov_trace_pc-0x4
 a98:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                        a9b: R_X86_64_32S       .data+0x3c0
 a9f:   4c 89 f6                mov    %r14,%rsi
 aa2:   e8 00 00 00 00          call   aa7 <memblock_bottom_up+0x67>
                        aa3: R_X86_64_PLT32
__ubsan_handle_load_invalid_value-0x4
 aa7:   eb cf                   jmp    a78 <memblock_bottom_up+0x38>
 aa9:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
 ab0:   00 00 00
 ab3:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
 aba:   00 00 00
 abd:   0f 1f 00                nopl   (%rax)

This means that the sanitiers added a lot of extra checking around what would have been a trivial global variable access otherwise. In this case, not inlining would be a reasonable decision.

Arnd

Signature · 2 lines
Sincerely yours,
Mike.
author
#5 lore raw
Quoted 10 lines · Mike Rapoport
Quoted 7 lines · Arnd Bergmann
Quoted 3 lines · David Hildenbrand

(I don't see why to not inline that function, but I am obviously not a compiler person :) )

Looking at the assembler output in the arm64 build that triggered the warning, I see this code:

"push %rbp" seems more x86 for me, but that's not really important :)

I suppose the relocation names like "R_X86_64_32S" and the command line I used should could have told me the same ;-)

Quoted 2 lines

I wonder what happens with other memblock inline APIs, particularly with alloc wrappers. Do they still get inlined?

Trying the same configuration here, with all the allocation functions marked __init again, they all get inlined by clang, regardless of the '__init' and 'inline' and '__always_inline' tags.

With gcc-7 and gcc-10 one instance of the plain 'memblock_alloc' does not get fully inlined if I revert the __always_inline back to plain inline:

.type memblock_alloc.constprop.0, @function
memblock_alloc.constprop.0:
.LASANPC4090:
pushq %rbp #
# include/linux/memblock.h:407: static inline __init void *memblock_alloc(phys_addr_t size, phys_addr_t align)
movq %rdi, %rbp # tmp84, size
# include/linux/memblock.h:409: return memblock_alloc_try_nid(size,
align, MEMBLOCK_LOW_LIMIT,

        call    __sanitizer_cov_trace_pc        #
        movq    %rbp, %rdi      # size,
        orl     $-1, %r8d       #,
        xorl    %ecx, %ecx      #
        xorl    %edx, %edx      #
        movl    $4096, %esi     #,

# include/linux/memblock.h:411: }
popq %rbp #
# include/linux/memblock.h:409: return memblock_alloc_try_nid(size,
align, MEMBLOCK_LOW_LIMIT,
jmp memblock_alloc_try_nid #
.size memblock_alloc.constprop.0, .-memblock_alloc.constprop.0

Apparently, this is an optimization for code size, as there are multiple callers in
kernel/dma/swiotlb.c and it can move the call to __sanitizer_cov_trace_pc into a single place here.

Arnd

maintainer Reviewed-by on v1 1/1
#6 lore raw
In reply to Arnd Bergmann
Quoted 20 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The inlining logic in clang-13 is rewritten to often not inline some functions that were inlined by all earlier compilers.

In case of the memblock interfaces, this exposed a harmless bug of a missing __init annotation:

WARNING: modpost: vmlinux.o(.text+0x507c0a): Section mismatch in reference from the function memblock_bottom_up() to the variable .meminit.data:memblock The function memblock_bottom_up() references
the variable __meminitdata memblock.
This is often because memblock_bottom_up lacks a __meminitdata
annotation or the annotation of memblock is wrong.

Interestingly, these annotations were present originally, but got removed with the explanation that the __init annotation prevents the function from getting inlined. I checked this again and found that while this is the case with clang, gcc (version 7 through 10, did not test others) does inline the functions regardless.

Quoted 7 lines

As the previous change was apparently intended to help the clang builds, reverting it to help the newer clang versions seems appropriate as well. gcc builds don't seem to care either way.

Fixes 5bdba520c1b3 ("mm: memblock: drop __init from memblock functions to make it inline")

Reference: 2cfb3665e864 ("include/linux/memblock.h: add __init to memblock_set_bottom_up()")

Signed-off-by Arnd Bergmann <[email protected]>

I thought it'll go via memblock tree but since Andrew has already took it

Reviewed-by Mike Rapoport <[email protected]>
Quoted 29 lines
---
 include/linux/memblock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/memblock.h +2 -2
index c88bc24e31aa..d13e3cd938b4 100644
@@ -460,7 +460,7 @@ static inline void memblock_free_late(phys_addr_t base, phys_addr_t size)
 /*
  * Set the allocation direction to bottom-up or top-down.
  */
-static inline void memblock_set_bottom_up(bool enable)
+static inline __init void memblock_set_bottom_up(bool enable)
 {
 	memblock.bottom_up = enable;
 }

@@ -470,7 +470,7 @@ static inline void memblock_set_bottom_up(bool enable)
  * if this is true, that said, memblock will allocate memory
  * in bottom-up direction.
  */
-static inline bool memblock_bottom_up(void)
+static inline __init bool memblock_bottom_up(void)
 {
 	return memblock.bottom_up;
 }
Signature · 1 line
2.29.2
Signature · 2 lines
Sincerely yours,
Mike.