Message-ID: <20230731160402.GB1823389@dev-arch.thelio-3990X>
Date:   Mon, 31 Jul 2023 09:04:02 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Tiezhu Yang <yangtiezhu@...ngson.cn>,
        linux-arm-kernel@...ts.infradead.org,
        linux-riscv@...ts.infradead.org, loongarch@...ts.linux.dev,
        Linux-Arch <linux-arch@...r.kernel.org>, bpf@...r.kernel.org,
        linux-perf-users@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, loongson-kernel@...ts.loongnix.cn
Subject: Re: [PATCH v3 1/2] asm-generic: Unify uapi bitsperlong.h for arm64,
 riscv and loongarch

On Sat, Jul 29, 2023 at 11:12:26PM +0200, Arnd Bergmann wrote:
> 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.

Ah, makes perfect sense.

> >> 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.

Yeah, I think you are correct.

> 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.

Agreed, although you do make a good point below that we would need the
fix in the stable trees, which adds additional complexity when it comes
to things like bisecting. It is already hard enough with all the various
clang behavior changes we have had to adapt to over the years...

> 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.

but this does not sound like it would be the end of the world, I do not
have to bisect too often and even if I have to, using a userspace
without these headers is generally possible.

Cheers,
Nathan

Powered by blists - more mailing lists