Re: [PATCH] um: don't use CONFIG_X86_{32,64} symbols on x86

[Anton Ivanov]

On 03/09/2021 09:40, Johannes Berg wrote:
> On Fri, 2021-09-03 at 08:27 +0000, David Laight wrote:
> > From: Johannes Berg
> > > Sent: 02 September 2021 09:28
> > >
> > > The CONFIG_X86_32 and CONFIG_X86_64 symbols are used by
> > > both "real" x86 architecture builds and ARCH=um today.
> > > However, clearly most people and places in the code are
> > > treating it as the architecture Kconfig (technically
> > > that's just CONFIG_X86) and use it to indicate that the
> > > architecture is X86 in 32- or 64-bit flavour.
> > >
> > > This has caused a fair amount of issues in the past,
> > > for example drivers not building because use x86 macros
> > > or similar under CONFIG_X86_{32,64} ifdef, and then we
> > > find build reports and add "!UML" to their Kconfig etc.
> > >
> > > However, this is error-prone and a kind of whack-a-mole
> > > game, even with the build bots reporting things.
> > I suspect you've just changed the 'mole'.
> Yeah, that thought occurred to me too.
>
>
> > You've now got lots of lines like:
> >
> > #if defined(CONFIG_X86_64) || defined(CONFIG_X86_64_UML)
> >
> > Missing off the UML define is going to cause the 32bit code
> > to get compiled by mistake - which is likely to be more
> > confusing that the places where you need to do special 'stuff'
> > for UML
> I'm not sure I agree though.
>
> Yes, I agree that I have a number of lines. But looking through them, we
> have new ones:
>   - AFS, and it fundamentally wants to know the arch. If it misses one,
>     well, that can also happen with any other arch.
>   - XFS/falloc has arch-specific stuff again, and considers other
>     architectures too
>   - fs/ioctl.c is actually unnecessary since CONFIG_COMPAT doesn't exist
>     on UML
>   - same for blktrace
>   - BPF jit - not really sure about that one
BPF jit IIRC works. People are using UML as a verifier in testing environments for the "does it load and run on this kernel version" test.
>   - crypto Kconfig - but again already considers different architectures
>     there
>   - sound - ok that's a stupid one :)
>   - lzo - again stuff that already considers many architectures
>
> But on the other side removal we have
>   - sysctl
>   - netfilter
>   - security
>   - many fixes to driver Kconfig that you don't see here because they're
>     *missing* "depends on !UML" now
>
>
> So my gut feeling is that while we've indeed traded one mole for another
> in a sense, the somewhat surprising places (like sound and BPF) are much
> fewer, and most of the places that we now need it are places that are
> already considering different architectures.

+1. I'd rather fix all places the mole pops up once and for all.

> johannes
>
>
> _______________________________________________
> linux-um mailing list
> linux-um@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/