| Message ID | 1687443219-11946-1-git-send-email-yangtiezhu@loongson.cn |
|---|---|
| Headers | show |
| Series | Unify uapi bitsperlong.h | expand |
On Thu, Jun 22, 2023 at 8:10 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Jun 22, 2023, at 16:13, Tiezhu Yang wrote: > > v3: > > -- Check the definition of __BITS_PER_LONG first at > > the beginning of uapi/asm-generic/bitsperlong.h > > Thanks for doing this cleanup! I just wanted to report an issue I ran into with building the Linux perf tool. The header guard in: tools/include/asm-generic/bitsperlong.h https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/include/asm-generic/bitsperlong.h Caused an issue with building: tools/perf/util/cs-etm.c https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/cs-etm.c The issue was that cs-etm.c would #include a system header, which would transitively include a header with the same header guard. This led to the tools/include/asm-generic/bitsperlong.h being ignored and the compilation of tools/perf/util/cs-etm.c failing due to a missing define. My local workaround is: ``` diff --git a/tools/include/asm-generic/bitsperlong.h b/tools/include/asm-generic/bitsperlong.h index 2093d56ddd11..88508a35cb45 100644 --- a/tools/include/asm-generic/bitsperlong.h +++ b/tools/include/asm-generic/bitsperlong.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 */ -#ifndef __ASM_GENERIC_BITS_PER_LONG -#define __ASM_GENERIC_BITS_PER_LONG +#ifndef __LINUX_TOOLS_ASM_GENERIC_BITS_PER_LONG +#define __LINUX_TOOLS_ASM_GENERIC_BITS_PER_LONG #include <uapi/asm-generic/bitsperlong.h> @@ -21,4 +21,4 @@ #define small_const_nbits(nbits) \ (__builtin_constant_p(nbits) && (nbits) <= BITS_PER_LONG && (nbits) > 0) -#endif /* __ASM_GENERIC_BITS_PER_LONG */ +#endif /* __LINUX_TOOLS_ASM_GENERIC_BITS_PER_LONG */ ``` I'm not sure if a wider fix is necessary for this, but I thought it worthwhile to report that there are potential issues. I don't think we can use #pragma once, as an alternative to header guards, to avoid this kind of name collision. Thanks, Ian > > v2: > > -- Check __CHAR_BIT__ and __SIZEOF_LONG__ rather than > > __aarch64__, __riscv, __loongarch__, thanks Ruoyao > > -- Update the code comment and commit message > > > > v1: > > -- Rebase on 6.4-rc6 > > -- Only unify uapi bitsperlong.h for arm64, riscv and loongarch > > -- Remove uapi bitsperlong.h of hexagon and microblaze in a new patch > > > > Here is the RFC patch: > > https://lore.kernel.org/linux-arch/1683615903-10862-1-git-send-email-yangtiezhu@loongson.cn/ > > I've applied these to the asm-generic tree now > > Thanks, > > Arnd
On Fri, Jul 14, 2023, at 20:34, Ian Rogers wrote: > On Thu, Jun 22, 2023 at 8:10 AM Arnd Bergmann <arnd@arndb.de> wrote: >> >> On Thu, Jun 22, 2023, at 16:13, Tiezhu Yang wrote: >> > v3: >> > -- Check the definition of __BITS_PER_LONG first at >> > the beginning of uapi/asm-generic/bitsperlong.h >> > > > Thanks for doing this cleanup! I just wanted to report an issue I ran > into with building the Linux perf tool. The header guard in: > tools/include/asm-generic/bitsperlong.h > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/include/asm-generic/bitsperlong.h > > Caused an issue with building: > tools/perf/util/cs-etm.c > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/cs-etm.c > > The issue was that cs-etm.c would #include a system header, which > would transitively include a header with the same header guard. This > led to the tools/include/asm-generic/bitsperlong.h being ignored and > the compilation of tools/perf/util/cs-etm.c failing due to a missing > define. My local workaround is: > > ``` > diff --git a/tools/include/asm-generic/bitsperlong.h > b/tools/include/asm-generic/bitsperlong.h > index 2093d56ddd11..88508a35cb45 100644 > --- a/tools/include/asm-generic/bitsperlong.h > +++ b/tools/include/asm-generic/bitsperlong.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > -#ifndef __ASM_GENERIC_BITS_PER_LONG > -#define __ASM_GENERIC_BITS_PER_LONG > +#ifndef __LINUX_TOOLS_ASM_GENERIC_BITS_PER_LONG > +#define __LINUX_TOOLS_ASM_GENERIC_BITS_PER_LONG > #include <uapi/asm-generic/bitsperlong.h> > @@ -21,4 +21,4 @@ > #define small_const_nbits(nbits) \ > (__builtin_constant_p(nbits) && (nbits) <= BITS_PER_LONG && (nbits) > 0) > -#endif /* __ASM_GENERIC_BITS_PER_LONG */ > +#endif /* __LINUX_TOOLS_ASM_GENERIC_BITS_PER_LONG */ > ``` > > I'm not sure if a wider fix is necessary for this, but I thought it > worthwhile to report that there are potential issues. I don't think we > can use #pragma once, as an alternative to header guards, to avoid > this kind of name collision. Thanks for the report! I think the correct fix is to update the tools/include/ headers to have the same change as the kernel itself. I don't know why we end up including both, that sounds like a separate issue but should normally be harmless as long as the contents are the same. Arnd
v3: -- Check the definition of __BITS_PER_LONG first at the beginning of uapi/asm-generic/bitsperlong.h v2: -- Check __CHAR_BIT__ and __SIZEOF_LONG__ rather than __aarch64__, __riscv, __loongarch__, thanks Ruoyao -- Update the code comment and commit message v1: -- Rebase on 6.4-rc6 -- Only unify uapi bitsperlong.h for arm64, riscv and loongarch -- Remove uapi bitsperlong.h of hexagon and microblaze in a new patch Here is the RFC patch: https://lore.kernel.org/linux-arch/1683615903-10862-1-git-send-email-yangtiezhu@loongson.cn/ Tiezhu Yang (2): asm-generic: Unify uapi bitsperlong.h for arm64, riscv and loongarch tools arch: Remove uapi bitsperlong.h of hexagon and microblaze arch/arm64/include/uapi/asm/bitsperlong.h | 24 ------------------- arch/loongarch/include/uapi/asm/bitsperlong.h | 9 -------- arch/riscv/include/uapi/asm/bitsperlong.h | 14 ----------- include/uapi/asm-generic/bitsperlong.h | 13 ++++++++++- tools/arch/arm64/include/uapi/asm/bitsperlong.h | 24 ------------------- tools/arch/hexagon/include/uapi/asm/bitsperlong.h | 27 ---------------------- .../arch/loongarch/include/uapi/asm/bitsperlong.h | 9 -------- .../arch/microblaze/include/uapi/asm/bitsperlong.h | 2 -- tools/arch/riscv/include/uapi/asm/bitsperlong.h | 14 ----------- tools/include/uapi/asm-generic/bitsperlong.h | 14 ++++++++++- tools/include/uapi/asm/bitsperlong.h | 6 ----- 11 files changed, 25 insertions(+), 131 deletions(-) delete mode 100644 arch/arm64/include/uapi/asm/bitsperlong.h delete mode 100644 arch/loongarch/include/uapi/asm/bitsperlong.h delete mode 100644 arch/riscv/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/arm64/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/hexagon/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/loongarch/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/microblaze/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/riscv/include/uapi/asm/bitsperlong.h