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

From: Johannes Berg
Date: Fri Sep 03 2021 - 04:40:38 EST


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

johannes