Re: [PATCH v5 1/2] lib/test_bitops: Add benchmark test for fns()

[Nathan Chancellor <nathan@kernel.org>]

Hi Kuan-Wei,

On Fri, May 03, 2024 at 09:34:28AM +0800, Kuan-Wei Chiu wrote:
> On Fri, May 03, 2024 at 08:49:00AM +0800, kernel test robot wrote:
> > Hi Kuan-Wei,
> > 
> > kernel test robot noticed the following build errors:
> > 
> > [auto build test ERROR on linus/master]
> > [also build test ERROR on v6.9-rc6 next-20240502]
> > [cannot apply to akpm-mm/mm-everything akpm-mm/mm-nonmm-unstable]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Kuan-Wei-Chiu/lib-test_bitops-Add-benchmark-test-for-fns/20240502-172638
> > base:   linus/master
> > patch link:    https://lore.kernel.org/r/20240502092443.6845-2-visitorckw%40gmail.com
> > patch subject: [PATCH v5 1/2] lib/test_bitops: Add benchmark test for fns()
> > config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240503/202405030808.UsoMKFNP-lkp@intel.com/config)
> > compiler: clang version 18.1.4 (https://github.com/llvm/llvm-project e6c3289804a67ea0bb6a86fadbe454dd93b8d855)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240503/202405030808.UsoMKFNP-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202405030808.UsoMKFNP-lkp@intel.com/
> > 
> > All errors (new ones prefixed by >>):
> > 
> > >> lib/test_bitops.c:56:39: error: variable 'tmp' set but not used [-Werror,-Wunused-but-set-variable]
> >       56 |         static volatile __used unsigned long tmp __initdata;
> >          |                                              ^
> >    1 error generated.
> > 
> > 
> > vim +/tmp +56 lib/test_bitops.c
> > 
> >     52	
> >     53	static int __init test_fns(void)
> >     54	{
> >     55		static unsigned long buf[10000] __initdata;
> >   > 56		static volatile __used unsigned long tmp __initdata;
> 
> I apologize for causing the compilation failure with clang. I'm not
> very familiar with clang and I'm not sure why something marked as
> __used would result in the warning mentioned above. Perhaps clang does
> not support attribute((used))? Is there a way to work around this
> issue?

It looks like __attribute__((__used__)) is not enough to stop clang from
warning, unlike GCC. I can likely fix that in clang if it is acceptable
to the clang maintainers (although more below on why this might be
intentional) but the warning will still need to be resolved for older
versions. Looking at the current clang source code and tests, it looks
like __attribute__((__unused__)) should silence the warning, which the
kernel has available as __always_unused or __maybe_unused, depending on
the context.

$ cat test.c
void foo(void)
{
        int a;
        a = 1;
}

void bar(void)
{
        static int b;
        b = 2;
}

void baz(void)
{
        static int c __attribute__((__used__));
        c = 3;
}

void quux(void)
{
        static int d __attribute__((__unused__));
        d = 4;
}

void foobar(void)
{
        static int e __attribute__((__used__)) __attribute__((__unused__));
        e = 1;
}

$ gcc -Wunused-but-set-variable -c -o /dev/null test.c
test.c: In function ‘foo’:
test.c:3:13: warning: variable ‘a’ set but not used [-Wunused-but-set-variable]
    3 |         int a;
      |             ^
test.c: In function ‘bar’:
test.c:9:20: warning: variable ‘b’ set but not used [-Wunused-but-set-variable]
    9 |         static int b;
      |                    ^

$ clang -fsyntax-only -Wunused-but-set-variable test.c
test.c:3:6: warning: variable 'a' set but not used [-Wunused-but-set-variable]
    3 |         int a;
      |             ^
test.c:9:13: warning: variable 'b' set but not used [-Wunused-but-set-variable]
    9 |         static int b;
      |                    ^
test.c:15:13: warning: variable 'c' set but not used [-Wunused-but-set-variable]
   15 |         static int c __attribute__((__used__));
      |                    ^
3 warnings generated.

I've attached a diff below that resolves the warning for me and it has
no code generation differences based on objdump. While having used and
unused attributes together might look unusual, reading the GCC attribute
manual makes it seem like these attributes fulfill similar yet different
roles, __unused__ prevents any unused warnings while __used__ forces the
variable to be emitted:

https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Common-Variable-Attributes.html#index-unused-variable-attribute

A strict reading of that does not make it seem like __used__ implies
disabling unused warnings, so I can understand why clang's behavior is
the way that it is.

Cheers,
Nathan

diff --git a/lib/test_bitops.c b/lib/test_bitops.c
index 5c627b525a48..28c91072cf85 100644
--- a/lib/test_bitops.c
+++ b/lib/test_bitops.c
@@ -53,7 +53,7 @@ static unsigned long order_comb_long[][2] = {
 static int __init test_fns(void)
 {
 	static unsigned long buf[10000] __initdata;
-	static volatile __used unsigned long tmp __initdata;
+	static volatile __always_unused __used unsigned long tmp __initdata;
 	unsigned int i, n;
 	ktime_t time;