mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: David Laight <David.Laight@ACULAB.COM>,
	"linux-um@lists.infradead.org" <linux-um@lists.infradead.org>
Cc: "x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] um: don't use CONFIG_X86_{32,64} symbols on x86
Date: Fri, 03 Sep 2021 10:40:23 +0200	[thread overview]
Message-ID: <30ab717a9ba1c2bb17e2a6948ff61d26c4430772.camel@sipsolutions.net> (raw)
In-Reply-To: <f177f8b287f14c93b04542d7ab4a8dfd@AcuMS.aculab.com>

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


  reply	other threads:[~2021-09-03  8:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02  8:28 Johannes Berg
2021-09-03  8:27 ` David Laight
2021-09-03  8:40   ` Johannes Berg [this message]
2021-09-03 10:05     ` Anton Ivanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=30ab717a9ba1c2bb17e2a6948ff61d26c4430772.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=David.Laight@ACULAB.COM \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®