Re: [PATCH v3 1/2] asm-generic: Unify uapi bitsperlong.h for arm64, riscv and loongarch

[Arnd Bergmann]

On Sat, Jul 29, 2023, at 19:46, Nathan Chancellor wrote:
> On Sat, Jul 29, 2023 at 09:59:23AM +0200, Arnd Bergmann wrote:
>> On Sat, Jul 29, 2023, at 01:44, Nathan Chancellor wrote:

>> 
>> in order to get the ORC definitions from asm/orc_types.h, but
>> then it looks like it also gets the uapi/asm/bitsperlong.h
>> header from there which contains
>> 
>> #if defined(__x86_64__) && !defined(__ILP32__)
>> # define __BITS_PER_LONG 64
>> #else
>> # define __BITS_PER_LONG 32
>> #endif
>> 
>> and this would set __BITS_PER_LONG to 32 on arm64.
>> 
>> However, I don't see this actually being included on my
>> machine. Can you dump the sorttable.c preprocessor output
>> with your setup, using -fdirectives-only, so we can see
>> which of the two (__BITS_PER_LONG or BITS_PER_LONG) is
>> actually wrong and triggers the sanity check?
>
> Sure thing, this is the output of:
>
>   $ gcc -I/linux-stable/tools/include 
> -I/linux-stable/tools/arch/x86/include -DUNWINDER_ORC_ENABLED -I 
> ./scripts -E -fdirectives-only /linux-stable/scripts/sorttable.c
>
> https://gist.github.com/nathanchance/d2c3e58230930317dc84aff80fef38bf

Ok, so what we get is that the system-wide
/usr/include/aarch64-linux-gnu/asm/bitsperlong.h

includes the source tree file 
tools/include/asm-generic/bitsperlong.h

which in the old kernels only has the "32" fallback value.

>> What I see on my machine is that both definitions come
>> from the local tools/include/ headers, not from the
>> installed system headers, so I'm still doing something
>> wrong in my installation:
>
> Just to make sure, you have the 6.5-rc1+ headers installed and you are
> attempting to build the host tools from an earlier Linux release than
> 6.5-rc1? I don't see a problem with building these host programs on
> mainline/6.5, I see this issue when building them in older stable
> releases (my reproduction so far has been on 6.4 but I see it when
> building all currently supported long term stable releases) when I have
> the 6.5-rc1+ headers installed.

Ok, I see. I missed that part of your description earlier.

>
> which seems to be where the mismatch is coming from?

Right, exactly.

>> ./tools/include/asm-generic/bitsperlong.h
>> #define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__)
>> 
>> Neither of these files actually contains the sanity
>> check in linux-6.5-rc3, and comparing these is clearly
>> nonsensical, as they are defined the same way (rather
>> than checking CONFIG_64BIT), but also I don't see why
>> there is both a uapi/ version and a non-uapi version
>> in what is meant to be a userspace header.
>
> May be worth looping in the tools/ folks, since that whole directory is
> rather special IMO...

I think the good news is that this only happens because
the tools/ directory contains a copy of the kernel headers
including that sanity check, and other user space won't
suffer from it because they don't contain copies of kernel
internal headers.

Reverting the change might still end up being the easiest way
out regardless, but it does seem like we should be able
to address this in the tools directory by making sure it doesn't
mix the installed headers with the local ones.

Part of the problem I think is that the installed 
/usr/include/asm-generic/int-ll64.h includes
/usr/include/aarch64-linux-gnu/asm/bitsperlong.h, so both
of them are the uapi headers, but this one has an
"include <asm-generic/bitsperlong.h>" that expects the
uapi version but instead gets the kernel version from
the tools directory. We could override this by adding
a working tools/include/asm-generic/bitsperlong.h header,
but that has to be backported to the stable kernels then.

       Arnd