linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linuxfoundation.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Guenter Roeck <linux@roeck-us.net>,
	LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org,  Uros Bizjak <ubizjak@gmail.com>,
	linux-sparse@vger.kernel.org, lkp@intel.com,
	 oe-kbuild-all@lists.linux.dev
Subject: Re: [patch 5/9] x86: Cure per CPU madness on UP
Date: Fri, 15 Mar 2024 16:23:08 -0700	[thread overview]
Message-ID: <CAHk-=wiP+XMGHr8NU13sSOG_oasNZN02O9_c1PzCJNG7+O-GPw@mail.gmail.com> (raw)
In-Reply-To: <87o7bfjeae.ffs@tglx>

On Fri, 15 Mar 2024 at 15:55, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Not really. The problem is that a SMP build can run on a UP machine w/o
> APIC or command line disables the APIC and will run into the exactly
> same problem. The only case where we know that it is impossible is when
> APIC support is disabled, which is silly but topic for a different
> discussion.

Oh, I agree - that was why I said that it shouldn't depend on a local
APIC on machines that may not even have one.

That "may not even have one" can still be a static option - we
technically allow 32-bit UP kernel to not enable X86_UP_APIC, although
it might be time to drop that option.

> So the proper thing to do is to check for num_possible_cpus() == 1 in
> that function.

I think that's _one_ proper thing. I still think that the deeper
problem is that it still looks at local apic rules even when those
rules are completely nonsensical.

For example, that MAX_LOCAL_APIC range test may not matter simply
because it's testing a constant value, but it still smells entirely
wrong to even check for that, when the system doesn't necessarily have
one.

So I think your patch may fix the immediate bug, but I think it's
still just a band-aid.

Either we should just make all machines look like they have the proper
local apic mappings, or we shouldn't look at any local apic rules AT
ALL.

So I'd rather see those apic_maps[] just be properly filled in.

> Sure you can argue that we could avoid it for SMP=n builds completely,
> but I think the right thing to do is to aim for removing CONFIG_SMP and
> make the UP build a subset of a generic SMP capable build which has
> CONFIG_NR_CPUS=1, i.e. num_possible_cpus() = 1. Why?

I wouldn't be entirely opposed to just doing that. UP has become
fairly irrelevant.

That said, UP is *not* entirely irrelevant on other architectures, and
if we drop UP support on x86, we'll be effectively dropping a lot of
coverage testing. The number of people who do cross-compilers is
pretty small.

End result: I'd *much* rather get rid of X86_UP_APIC and the "nolapic"
kernel command line, and say "even UP has to have a local APIC".

We already require a Pentium-class CPU, so in practice we already
require that local APIC setup. And yes, machines existed where it
could be turned off, but I don't think that is relevant any more.

Put another way: I think "UP config for wider build testing" is a
_lot_ more relevant than "no LAPIC support".

             Linus

  reply	other threads:[~2024-03-15 23:23 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-04 10:12 [patch 0/9] x86: Cure tons of sparse warnings (mostly __percpu) Thomas Gleixner
2024-03-04 10:12 ` [patch 1/9] perf/x86/amd/uncore: Fix __percpu annotation Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] " tip-bot2 for Thomas Gleixner
2024-03-04 10:12 ` [patch 2/9] x86/msr: Prepare for including percpu.h Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] x86/msr: Prepare for including <linux/percpu.h> into <asm/msr.h> tip-bot2 for Thomas Gleixner
2024-03-04 10:12 ` [patch 3/9] x86/msr: Add missing __percpu annotations Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] " tip-bot2 for Thomas Gleixner
2024-03-04 10:12 ` [patch 4/9] smp: Consolidate smp_prepare_boot_cpu() Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] " tip-bot2 for Thomas Gleixner
2024-03-04 10:12 ` [patch 5/9] x86: Cure per CPU madness on UP Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] x86/percpu: " tip-bot2 for Thomas Gleixner
2024-03-15 16:17   ` [patch 5/9] x86: " Guenter Roeck
2024-03-15 16:42     ` Linus Torvalds
2024-03-15 17:02       ` Guenter Roeck
2024-03-15 17:40       ` Thomas Gleixner
2024-03-15 22:55         ` Thomas Gleixner
2024-03-15 23:23           ` Linus Torvalds [this message]
2024-03-16  1:11             ` Thomas Gleixner
2024-03-16  1:23               ` Linus Torvalds
2024-03-16 21:34                 ` Arnd Bergmann
2024-03-17 21:03               ` David Laight
2024-03-18 11:11               ` Thomas Gleixner
2024-03-18 17:27               ` Thomas Gleixner
2024-03-18 19:13                 ` Arnd Bergmann
2024-03-19 16:21                   ` Thomas Gleixner
2024-03-19 18:26                     ` Guenter Roeck
2024-03-16  0:56           ` Guenter Roeck
2024-03-20  8:58     ` Thomas Gleixner
2024-03-20 15:46       ` Guenter Roeck
2024-03-21 11:14         ` Thomas Gleixner
2024-03-21 14:06           ` Guenter Roeck
2024-03-21 16:49             ` Thomas Gleixner
2024-03-04 10:12 ` [patch 6/9] x86/uaccess: Add missing __force to casts in __access_ok() and valid_user_address() Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] " tip-bot2 for Thomas Gleixner
2024-03-04 10:12 ` [patch 7/9] x86/cpu: Use EXPORT_PER_CPU_SYMBOL_GPL() for x86_spec_ctrl_current Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] " tip-bot2 for Thomas Gleixner
2024-03-04 10:12 ` [patch 8/9] x86/cpu: Provide a declaration for itlb_multihit_kvm_mitigation Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] " tip-bot2 for Thomas Gleixner
2024-03-04 10:12 ` [patch 9/9] x86/callthunks: Use EXPORT_PER_CPU_SYMBOL_GPL() for per CPU variables Thomas Gleixner
2024-03-04 12:15   ` [tip: x86/cleanups] " tip-bot2 for Thomas Gleixner
2024-03-04 11:08 ` [patch 0/9] x86: Cure tons of sparse warnings (mostly __percpu) Ingo Molnar

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='CAHk-=wiP+XMGHr8NU13sSOG_oasNZN02O9_c1PzCJNG7+O-GPw@mail.gmail.com' \
    --to=torvalds@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lkp@intel.com \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=tglx@linutronix.de \
    --cc=ubizjak@gmail.com \
    --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;
as well as URLs for NNTP newsgroup(s).