Message-Id: <1be19a7f-f43c-4025-8cf9-5f831c4125f5@app.fastmail.com>
Date: Mon, 10 Mar 2025 13:50:28 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Thomas Huth" <thuth@...hat.com>, "Heiko Carstens" <hca@...ux.ibm.com>
Cc: "Vasily Gorbik" <gor@...ux.ibm.com>,
 "Alexander Gordeev" <agordeev@...ux.ibm.com>, linux-s390@...r.kernel.org,
 "Christian Borntraeger" <borntraeger@...ux.ibm.com>,
 "Sven Schnelle" <svens@...ux.ibm.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] s390/uapi: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

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

Powered by blists - more mailing lists