linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Barry Song <21cnbao@gmail.com>
To: gregkh@linuxfoundation.org, yury.norov@gmail.com,
	andriy.shevchenko@linux.intel.com, linux@rasmusvillemoes.dk,
	linux-kernel@vger.kernel.org
Cc: linuxarm@huawei.com, Barry Song <song.bao.hua@hisilicon.com>,
	kernel test robot <lkp@intel.com>,
	Max Filippov <jcmvbkbc@gmail.com>,
	Andrew Pinski <pinskia@gmail.com>
Subject: [PATCH] lib: bitmap: Mute some odd section mismatch warning in xtensa kernel build
Date: Sun, 15 Aug 2021 15:21:32 +1200	[thread overview]
Message-ID: <20210815032132.14530-1-21cnbao@gmail.com> (raw)

From: Barry Song <song.bao.hua@hisilicon.com>

Constanly there are some section mismatch issues reported in test_bitmap
for xtensa platform such as:

  Section mismatch in reference from the function bitmap_equal() to the
  variable .init.data:initcall_level_names
  The function bitmap_equal() references the variable __initconst
  __setup_str_initcall_blacklist. This is often because bitmap_equal
  lacks a __initconst annotation or the annotation of
  __setup_str_initcall_blacklist is wrong.

  Section mismatch in reference from the function bitmap_copy_clear_tail()
  to the variable .init.rodata:__setup_str_initcall_blacklist
  The function bitmap_copy_clear_tail() references the variable __initconst
  __setup_str_initcall_blacklist.
  This is often because bitmap_copy_clear_tail lacks a __initconst
  annotation or the annotation of __setup_str_initcall_blacklist is wrong.

To be honest, hardly to believe kernel code is wrong since bitmap_equal is
always called in __init function in test_bitmap.c just like __bitmap_equal.
But gcc doesn't report any issue for __bitmap_equal even when bitmap_equal
and __bitmap_equal show in the same function such as:

  static void noinline __init test_mem_optimisations(void)
  {
	...
          for (start = 0; start < 1024; start += 8) {
                  for (nbits = 0; nbits < 1024 - start; nbits += 8) {
                          if (!bitmap_equal(bmap1, bmap2, 1024)) {
                                  failed_tests++;
                          }
                          if (!__bitmap_equal(bmap1, bmap2, 1024)) {
                                  failed_tests++;
                          }
  			...
                  }
          }
  }

The different between __bitmap_equal() and bitmap_equal() is that the
former is extern and a EXPORT_SYMBOL. So noinline, and probably in fact
noclone. But the later is static and unfortunately not inlined at this
time though it has a "inline" flag.

bitmap_copy_clear_tail(), on the other hand, seems more innocent as it is
accessing stack only by its wrapper bitmap_from_arr32() in function
test_bitmap_arr32():
static void __init test_bitmap_arr32(void)
{
        unsigned int nbits, next_bit;
        u32 arr[EXP1_IN_BITS / 32];
        DECLARE_BITMAP(bmap2, EXP1_IN_BITS);

        memset(arr, 0xa5, sizeof(arr));

        for (nbits = 0; nbits < EXP1_IN_BITS; ++nbits) {
                bitmap_to_arr32(arr, exp1, nbits);
                bitmap_from_arr32(bmap2, arr, nbits);
                expect_eq_bitmap(bmap2, exp1, nbits);
		...
        }
}
Looks like gcc optimized arr, bmap2 things to .init.data but it seems
nothing is wrong in kernel since test_bitmap_arr32() is __init.

Max Filippov reported a bug to gcc but gcc people don't ack. So here
this patch removes the involved symbols by forcing inline. It might
not be that elegant but I don't see any harm as bitmap_equal() and
bitmap_copy_clear_tail() are both quite small. In addition, kernel
doc also backs this modification "We don't use the 'inline' keyword
because it's broken": www.kernel.org/doc/local/inline.html

Another possible way to "fix" the warning is moving the involved
symboms to lib/bitmap.c:

  +int bitmap_equal(const unsigned long *src1,
  +                       const unsigned long *src2, unsigned int nbits)
  +{
  +       if (small_const_nbits(nbits))
  +               return !((*src1 ^ *src2) & BITMAP_LAST_WORD_MASK(nbits));
  +       if (__builtin_constant_p(nbits & BITMAP_MEM_MASK) &&
  +           IS_ALIGNED(nbits, BITMAP_MEM_ALIGNMENT))
  +               return !memcmp(src1, src2, nbits / 8);
  +       return __bitmap_equal(src1, src2, nbits);
  +}
  +EXPORT_SYMBOL(bitmap_equal);

This is harmful to the performance.

Reported-by: kernel test robot <lkp@intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Andrew Pinski <pinskia@gmail.com>
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92938
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 include/linux/bitmap.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 37f36dad18bd..3eec9f68a0b6 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -258,7 +258,7 @@ static inline void bitmap_copy(unsigned long *dst, const unsigned long *src,
 /*
  * Copy bitmap and clear tail bits in last word.
  */
-static inline void bitmap_copy_clear_tail(unsigned long *dst,
+static __always_inline void bitmap_copy_clear_tail(unsigned long *dst,
 		const unsigned long *src, unsigned int nbits)
 {
 	bitmap_copy(dst, src, nbits);
@@ -334,7 +334,7 @@ static inline void bitmap_complement(unsigned long *dst, const unsigned long *sr
 #endif
 #define BITMAP_MEM_MASK (BITMAP_MEM_ALIGNMENT - 1)
 
-static inline int bitmap_equal(const unsigned long *src1,
+static __always_inline int bitmap_equal(const unsigned long *src1,
 			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
-- 
2.25.1


             reply	other threads:[~2021-08-15  3:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-15  3:21 Barry Song [this message]
2021-08-15 10:30 ` Andy Shevchenko
2021-08-15 11:25   ` Barry Song
2021-08-15 15:55 ` Yury Norov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210815032132.14530-1-21cnbao@gmail.com \
    --to=21cnbao@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jcmvbkbc@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=linuxarm@huawei.com \
    --cc=lkp@intel.com \
    --cc=pinskia@gmail.com \
    --cc=song.bao.hua@hisilicon.com \
    --cc=yury.norov@gmail.com \
    --subject='Re: [PATCH] lib: bitmap: Mute some odd section mismatch warning in xtensa kernel build' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).