On Tue, May 11, 2021 at 08:53:53PM +0900, Tetsuo Handa wrote:

On 2021/05/11 0:44, Andy Shevchenko wrote:

quoted

And I'm a bit lost here, because I can't imagine the offset being
constant along with a size of bitmap. What do we want to achieve by
this? Any examples to better understand the case?

Because I feel that the GENMASK() macro cannot be evaluated without
both arguments being a constant.

The usage is

 unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
                            unsigned long offset)
 {
+       if (small_const_nbits(size)) {
+               unsigned long val;
+
+               if (unlikely(offset >= size))
+                       return size;
+
+               val = *addr & GENMASK(size - 1, offset);
+               return val ? __ffs(val) : size;
+       }
+
        return _find_next_bit(addr, NULL, size, offset, 0UL, 0);
 }

where GENMASK() might be called even if "offset" is not a constant.

#define GENMASK(h, l) \
     (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))

#define __GENMASK(h, l) \
     (((~UL(0)) - (UL(1) << (l)) + 1) & \
       (~UL(0) >> (BITS_PER_LONG - 1 - (h))))

#define GENMASK_INPUT_CHECK(h, l) \
     (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
          __builtin_constant_p((l) > (h)), (l) > (h), 0)))

__GENMASK() does not need "h" and "l" being a constant.

Yes, small_const_nbits(size) in find_next_bit() can guarantee that "size" is a
constant and hence "h" argument in GENMASK_INPUT_CHECK() call is also a constant.
But nothing can guarantee that "offset" is a constant, and hence nothing can
guarantee that "l" argument in GENMASK_INPUT_CHECK() call is also a constant.

Then, how can (l) > (h) in __builtin_constant_p((l) > (h)) be evaluated at build time
if either l or h (i.e. "offset" and "size - 1" in find_next_bit()) lacks a guarantee of
being a constant?

So the idea is that if (l > h) is constant, __builtin_constant_p should
evaluate that, and if it is not it should use zero instead as input to
__builtin_chose_expr(). This works with non-const inputs in many other
places in the kernel, but apparently in this case with a certain
compiler, it doesn't so I guess we need to work around it.

But what a surprise,

On 2021/05/11 7:51, Rikard Falkeborn wrote:

quoted

Does the following work for you? For simplicity, I copied__is_constexpr from
include/linux/minmax.h (which isn't available in tools/). A proper patch
would reuse __is_constexpr (possibly refactoring it to a separate
header since bits.h including minmax.h for that only seems smelly) and fix
bits.h in the kernel header as well, to keep the files in sync.

this works for me.

Great, thanks for testing!

I sent a patch for this here:
https://lore.kernel.org/lkml/20210511203716.117010-1-rikard.falkeborn@gmail.com/ (local)

Rikard

quoted

diff --git a/tools/include/linux/bits.h b/tools/include/linux/bits.h
index 7f475d59a097..7bc4c31a7df0 100644
--- a/tools/include/linux/bits.h
+++ b/tools/include/linux/bits.h

@@ -19,10 +19,13 @@
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
 #if !defined(__ASSEMBLY__)
+
+#define __is_constexpr(x) \
+       (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
 #include <linux/build_bug.h>
 #define GENMASK_INPUT_CHECK(h, l) \
        (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
-               __builtin_constant_p((l) > (h)), (l) > (h), 0)))
+               __is_constexpr((l) > (h)), (l) > (h), 0)))
 #else
 /*
  * BUILD_BUG_ON_ZERO is not available in h files included from asm files,

On 2021/05/11 7:52, Yury Norov wrote:

quoted

I tested the objtool build with the 8.4.0 and 7.5.0 compilers from
ubuntu 21 distro, and it looks working. Can you please share more
details about your system? 

Nothing special. A plain x86_64 CentOS 7.9 system with devtoolset-8.

$ /opt/rh/devtoolset-8/root/bin/gcc --version
gcc (GCC) 8.3.1 20190311 (Red Hat 8.3.1-3)
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ rpm -qi devtoolset-8-gcc
Name        : devtoolset-8-gcc
Version     : 8.3.1
Release     : 3.2.el7
Architecture: x86_64
Install Date: Wed Apr 22 07:58:16 2020
Group       : Development/Languages
Size        : 74838011
License     : GPLv3+ and GPLv3+ with exceptions and GPLv2+ with exceptions and LGPLv2+ and BSD
Signature   : RSA/SHA1, Thu Apr 16 19:44:43 2020, Key ID 4eb84e71f2ee9d55
Source RPM  : devtoolset-8-gcc-8.3.1-3.2.el7.src.rpm
Build Date  : Sat Mar 28 00:06:45 2020
Build Host  : c1be.rdu2.centos.org
Relocations : (not relocatable)
Packager    : CBS [off-list ref]
Vendor      : CentOS
URL         : http://gcc.gnu.org
Summary     : GCC version 8
Description :
The devtoolset-8-gcc package contains the GNU Compiler Collection version 7.