arch/s390/include/uapi/asm/ptrace.h | 5 +++-- arch/s390/include/uapi/asm/schid.h | 4 ++-- arch/s390/include/uapi/asm/types.h | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-)
__ASSEMBLY__ is only defined by the Makefile of the kernel, so
this is not really useful for uapi headers (unless the userspace
Makefile defines it, too). Let's switch to __ASSEMBLER__ which
gets set automatically by the compiler when compiling assembly
code.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
arch/s390/include/uapi/asm/ptrace.h | 5 +++--
arch/s390/include/uapi/asm/schid.h | 4 ++--
arch/s390/include/uapi/asm/types.h | 4 ++--
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h
index bb0826024bb95..ea202072f1ad5 100644
--- a/arch/s390/include/uapi/asm/ptrace.h
+++ b/arch/s390/include/uapi/asm/ptrace.h
@@ -242,7 +242,8 @@
#define PTRACE_OLDSETOPTIONS 21
#define PTRACE_SYSEMU 31
#define PTRACE_SYSEMU_SINGLESTEP 32
-#ifndef __ASSEMBLY__
+
+#ifndef __ASSEMBLER__
#include <linux/stddef.h>
#include <linux/types.h>
@@ -450,6 +451,6 @@ struct user_regs_struct {
unsigned long ieee_instruction_pointer; /* obsolete, always 0 */
};
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
#endif /* _UAPI_S390_PTRACE_H */
diff --git a/arch/s390/include/uapi/asm/schid.h b/arch/s390/include/uapi/asm/schid.h
index a3e1cf1685534..d804d1a5b1b3f 100644
--- a/arch/s390/include/uapi/asm/schid.h
+++ b/arch/s390/include/uapi/asm/schid.h
@@ -4,7 +4,7 @@
#include <linux/types.h>
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
struct subchannel_id {
__u32 cssid : 8;
@@ -15,6 +15,6 @@ struct subchannel_id {
__u32 sch_no : 16;
} __attribute__ ((packed, aligned(4)));
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
#endif /* _UAPIASM_SCHID_H */
diff --git a/arch/s390/include/uapi/asm/types.h b/arch/s390/include/uapi/asm/types.h
index 84457dbb26b4a..4ab468c5032e3 100644
--- a/arch/s390/include/uapi/asm/types.h
+++ b/arch/s390/include/uapi/asm/types.h
@@ -10,7 +10,7 @@
#include <asm-generic/int-ll64.h>
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
typedef unsigned long addr_t;
typedef __signed__ long saddr_t;
@@ -25,6 +25,6 @@ typedef struct {
};
} __attribute__((packed, aligned(4))) __vector128;
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
#endif /* _UAPI_S390_TYPES_H */
--
2.48.1On Mon, Mar 10, 2025 at 11:26:57AM +0100, Thomas Huth wrote: > __ASSEMBLY__ is only defined by the Makefile of the kernel, so > this is not really useful for uapi headers (unless the userspace > Makefile defines it, too). Let's switch to __ASSEMBLER__ which > gets set automatically by the compiler when compiling assembly > code. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > arch/s390/include/uapi/asm/ptrace.h | 5 +++-- > arch/s390/include/uapi/asm/schid.h | 4 ++-- > arch/s390/include/uapi/asm/types.h | 4 ++-- > 3 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h > index bb0826024bb95..ea202072f1ad5 100644 > --- a/arch/s390/include/uapi/asm/ptrace.h > +++ b/arch/s390/include/uapi/asm/ptrace.h > @@ -242,7 +242,8 @@ > #define PTRACE_OLDSETOPTIONS 21 > #define PTRACE_SYSEMU 31 > #define PTRACE_SYSEMU_SINGLESTEP 32 > -#ifndef __ASSEMBLY__ > + > +#ifndef __ASSEMBLER__ > #include <linux/stddef.h> > #include <linux/types.h> ... Did this cause any sorts of problems? I can see this pattern all over the place, so why is this now a problem? Also, wouldn't it be better to fix this with an sed statement in scripts/headers_install.sh instead? Otherwise this is going to be a never ending story since those things will be re-introduced all the time.
On Mon, Mar 10, 2025, at 11:49, Heiko Carstens wrote: > On Mon, Mar 10, 2025 at 11:26:57AM +0100, Thomas Huth wrote: > > Did this cause any sorts of problems? I can see this pattern all over > the place, so why is this now a problem? > > Also, wouldn't it be better to fix this with an sed statement in > scripts/headers_install.sh instead? Otherwise this is going to be a > never ending story since those things will be re-introduced all the > time. It should certainly be done in a consistent way across all architectures and architecture-independent headers. I see that all uapi headers use __ASSEMBLY__ consistently, while a few non-uapi headers use __ASSEMBLER__. glibc obviously defines __ASSEMBLY__ whenever it includes one of the kernel headers that need this from a .S file. Unless there is a known problem with the current code, leaving this unchanged is probably the least risky way. Arnd
On 10/03/2025 12.07, Arnd Bergmann wrote: > On Mon, Mar 10, 2025, at 11:49, Heiko Carstens wrote: >> On Mon, Mar 10, 2025 at 11:26:57AM +0100, Thomas Huth wrote: >> >> Did this cause any sorts of problems? I can see this pattern all over >> the place, so why is this now a problem? >> >> Also, wouldn't it be better to fix this with an sed statement in >> scripts/headers_install.sh instead? Otherwise this is going to be a >> never ending story since those things will be re-introduced all the >> time. > > It should certainly be done in a consistent way across all > architectures and architecture-independent headers. I see that > all uapi headers use __ASSEMBLY__ consistently, while a few non-uapi > headers use __ASSEMBLER__. > > glibc obviously defines __ASSEMBLY__ whenever it includes one > of the kernel headers that need this from a .S file. Unless > there is a known problem with the current code, leaving this > unchanged is probably the least risky way. Well, this seems to be constant source of confusion. It got my attention by Sean's recent patch for kvm-unit-tests here: https://lore.kernel.org/kvm/20250222014526.2302653-1-seanjc@google.com/ Quoting: "This is essentially a "rage" patch after spending way, way too much time trying to understand why I couldn't include some __ASSEMBLY__ protected headers in x86 assembly files." But also if you search the net for this, there are lots of other spots where people get it wrong, e.g.: https://stackoverflow.com/questions/28924355/gcc-assembler-preprocessor-not-compatible-with-standard-headers https://forums.raspberrypi.com/viewtopic.php?p=1652944#p1653834 https://github.com/riscv-software-src/opensbi/issues/199 So I thought it would be a good idea to standardize on the #define that is set by the compiler already. IMHO it would be great to get it replaced in the whole kernel, but that's a little bit bold for one patch. So the obvious first step towards that direction is to replace it in the uapi header files first, where it hopefully will help to reduce the confusion in userspace. So unless you really don't like this idea at all, I could continue with the uapi headers for the other architectures, too? Thomas
On Mon, Mar 10, 2025, at 13:14, Thomas Huth wrote:
> On 10/03/2025 12.07, Arnd Bergmann wrote:
>> On Mon, Mar 10, 2025, at 11:49, Heiko Carstens wrote:
>>> On Mon, Mar 10, 2025 at 11:26:57AM +0100, Thomas Huth wrote:
>>>
>>> Did this cause any sorts of problems? I can see this pattern all over
>>> the place, so why is this now a problem?
>>>
>>> Also, wouldn't it be better to fix this with an sed statement in
>>> scripts/headers_install.sh instead? Otherwise this is going to be a
>>> never ending story since those things will be re-introduced all the
>>> time.
>>
>> It should certainly be done in a consistent way across all
>> architectures and architecture-independent headers. I see that
>> all uapi headers use __ASSEMBLY__ consistently, while a few non-uapi
>> headers use __ASSEMBLER__.
>>
>> glibc obviously defines __ASSEMBLY__ whenever it includes one
>> of the kernel headers that need this from a .S file. Unless
>> there is a known problem with the current code, leaving this
>> unchanged is probably the least risky way.
>
> Well, this seems to be constant source of confusion. It got my attention by
> Sean's recent patch for kvm-unit-tests here:
>
> https://lore.kernel.org/kvm/20250222014526.2302653-1-seanjc@google.com/
>
> Quoting: "This is essentially a "rage" patch after spending
> way, way too much time trying to understand why I couldn't include some
> __ASSEMBLY__ protected headers in x86 assembly files."
>
> But also if you search the net for this, there are lots of other spots where
> people get it wrong, e.g.:
>
>
> https://stackoverflow.com/questions/28924355/gcc-assembler-preprocessor-not-compatible-with-standard-headers
> https://forums.raspberrypi.com/viewtopic.php?p=1652944#p1653834
> https://github.com/riscv-software-src/opensbi/issues/199
Right
> So I thought it would be a good idea to standardize on the #define that is
> set by the compiler already. IMHO it would be great to get it replaced in
> the whole kernel, but that's a little bit bold for one patch. So the obvious
> first step towards that direction is to replace it in the uapi header files
> first, where it hopefully will help to reduce the confusion in userspace.
> So unless you really don't like this idea at all, I could continue with the
> uapi headers for the other architectures, too?
Standardizing on one of the two is good, and using the one that the
toolchain already provides makes sense. I would prefer a large patch
that replaces all of them (uapi and internal) and removes the
definition at the same time, the way the kvm patch does, but it's
possible that this causes conflicts with architecture specific
patches.
There is a risk of regression when changing the uapi headers, when
new users of the uapi headers don't define __ASSEMBLER__:
This would then work with new kernel headers but not when building
against older headers.
There is also a (smaller) risk when there is userland building
assembler files with a compiler other than gcc or clang, if they set
__ASSEMBLY__ manually but the compiler does not set __ASSEMBLER__.
I checked that gcc has defined __ASSEMBLER__ at least as far back
as egcs-1.0.0 and gcc-2.95, probably longer.
Arnd
© 2016 - 2026 Red Hat, Inc.