mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: Josh Triplett <josh@joshtriplett.org>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Usama Arif <usama.arif@bytedance.com>,
	dwmw2@infradead.org, kim.phillips@amd.com, brgerst@gmail.com,
	piotrgorski@cachyos.org, oleksandr@natalenko.name,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	hpa@zytor.com, x86@kernel.org, pbonzini@redhat.com,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	rcu@vger.kernel.org, mimoja@mimoja.de, hewenliang4@huawei.com,
	thomas.lendacky@amd.com, seanjc@google.com,
	pmenzel@molgen.mpg.de, fam.zheng@bytedance.com,
	punit.agrawal@bytedance.com, simon.evans@bytedance.com,
	liangma@liangbit.com, David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH v12 07/11] x86/smpboot: Remove early_gdt_descr on 64-bit
Date: Wed, 1 Mar 2023 17:37:24 -0800	[thread overview]
Message-ID: <20230302013724.GO2948950@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <41baeedf-f3ee-7342-7a5e-097f9a3c4de0@infradead.org>

On Wed, Mar 01, 2023 at 05:05:39PM -0800, Randy Dunlap wrote:
> 
> 
> On 3/1/23 16:28, Paul E. McKenney wrote:
> > On Wed, Mar 01, 2023 at 02:25:36PM -0800, Josh Triplett wrote:
> >> On Wed, Mar 01, 2023 at 02:16:32PM -0800, Paul E. McKenney wrote:
> >>> On Wed, Mar 01, 2023 at 12:32:26PM -0800, Josh Triplett wrote:
> >>>> On Tue, Feb 28, 2023 at 01:02:33PM -0800, Arjan van de Ven wrote:
> >>>>   Thomas Gleixner wrote:
> >>>>>>
> >>>>>> Maybe we should enforce CONFIG_SMP=y first :)
> >>>>>>
> >>>>>> Thanks,
> >>>>>
> >>>>> for 64 bit I can see the point of removing the !SMP case entirely from arch/x86 .
> >>>>> maybe even for 32 bit if it just makes the code simpler I suppose
> >>>>
> >>>> As one of the folks keeping an eye on tinyconfig and kernel size, I
> >>>> actually think we *should* make this change and rip out !CONFIG_SMP,
> >>>> albeit carefully.
> >>>>
> >>>> In particular, I would propose that we rip out !CONFIG_SMP, *but* we
> >>>> allow building with CONFIG_NR_CPUS=1. (And we could make sure in that
> >>>> case that the compiler can recognize that at compile time and optimize
> >>>> accordingly, so that it might provide some of the UP optimizations for
> >>>> us.)
> >>>>
> >>>> Then, any *optimizations* for the "will only have one CPU, ever" case
> >>>> can move to CONFIG_NR_CPUS=1 rather than !CONFIG_SMP. I think many of
> >>>> those optimizations may be worth keeping for small embedded systems, or
> >>>> for cases like Linux-as-bootloader or similar.
> >>>>
> >>>> The difference here would be that code written for !CONFIG_SMP today
> >>>> needs to account for the UP case for *correctness*, whereas code written
> >>>> for CONFIG_SMP can *optionally* consider CONFIG_NR_CPUS=1 for
> >>>> *performance*.
> >>>
> >>> It certainly would not make much sense to keep Tiny RCU and Tiny SRCU
> >>> around if there was no CONFIG_SMP=n.
> >>
> >> On the contrary, I think it's entirely appropriate to keep them for
> >> CONFIG_NR_CPUS=1; that's exactly the kind of simple optimization that
> >> seems well worth having. (Ideal optimization: "very very simple for UP,
> >> complex for SMP"; non-ideal optimization: "complex for SMP, differently
> >> complex for UP".)
> > 
> > Fair enough, but how does removing CONFIG_SMP help with that?  Given that
> > it is not all that hard to work around the lack of CONFIG_SMP for Tiny
> > RCU and Tiny SRCU, then it cannot be all that hard to work around that
> > lack for the use cases that you are trying to get rid of, right?
> > 
> > 							Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> > diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> > index 9071182b1284b..7487bee3d4341 100644
> > --- a/kernel/rcu/Kconfig
> > +++ b/kernel/rcu/Kconfig
> > @@ -7,7 +7,7 @@ menu "RCU Subsystem"
> >  
> >  config TREE_RCU
> >  	bool
> > -	default y if SMP
> > +	default y if CONFIG_NR_CPUS = 1
> >  	# Dynticks-idle tracking
> >  	select CONTEXT_TRACKING_IDLE
> >  	help
> > @@ -31,7 +31,7 @@ config PREEMPT_RCU
> >  
> >  config TINY_RCU
> >  	bool
> > -	default y if !PREEMPTION && !SMP
> > +	default y if !PREEMPTION && CONFIG_NR_CPUS != 1
> >  	help
> >  	  This option selects the RCU implementation that is
> >  	  designed for UP systems from which real-time response
> 
> but drop the CONFIG_ prefixes...

Indeed.  What I don't understand is how the above passed some light
rcutorture testing.  And the comparisons are backwards as well.
Perhaps this time two bugs make working code?

How about the following?

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
index 9071182b1284b..a2ba97b949498 100644
--- a/kernel/rcu/Kconfig
+++ b/kernel/rcu/Kconfig
@@ -7,7 +7,7 @@ menu "RCU Subsystem"
 
 config TREE_RCU
 	bool
-	default y if SMP
+	default y if NR_CPUS != 1
 	# Dynticks-idle tracking
 	select CONTEXT_TRACKING_IDLE
 	help
@@ -31,7 +31,7 @@ config PREEMPT_RCU
 
 config TINY_RCU
 	bool
-	default y if !PREEMPTION && !SMP
+	default y if !PREEMPTION && NR_CPUS = 1
 	help
 	  This option selects the RCU implementation that is
 	  designed for UP systems from which real-time response

  reply	other threads:[~2023-03-02  1:37 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-26 11:07 [PATCH v12 00/11] Parallel CPU bringup for x86_64 Usama Arif
2023-02-26 11:07 ` [PATCH v12 01/11] x86/apic/x2apic: Allow CPU cluster_mask to be populated in parallel Usama Arif
2023-02-26 11:07 ` [PATCH v12 02/11] cpu/hotplug: Move idle_thread_get() to <linux/smpboot.h> Usama Arif
2023-02-26 11:07 ` [PATCH v12 03/11] cpu/hotplug: Add dynamic parallel bringup states before CPUHP_BRINGUP_CPU Usama Arif
2023-02-26 11:07 ` [PATCH v12 04/11] x86/smpboot: Reference count on smpboot_setup_warm_reset_vector() Usama Arif
2023-02-26 11:07 ` [PATCH v12 05/11] x86/smpboot: Split up native_cpu_up into separate phases and document them Usama Arif
2023-02-26 11:07 ` [PATCH v12 06/11] x86/smpboot: Remove initial_stack on 64-bit Usama Arif
2023-02-28 16:13   ` Thomas Gleixner
2023-02-28 16:25     ` David Woodhouse
2023-02-28 20:32       ` Thomas Gleixner
2023-02-28 17:09     ` David Woodhouse
2023-02-28 20:17       ` Thomas Gleixner
2023-02-28 20:43         ` David Woodhouse
2023-03-01 10:18           ` Thomas Gleixner
2023-03-01 10:42             ` David Woodhouse
2023-02-28 22:22     ` Brian Gerst
2023-02-26 11:07 ` [PATCH v12 07/11] x86/smpboot: Remove early_gdt_descr " Usama Arif
2023-02-28 21:01   ` Thomas Gleixner
2023-02-28 21:02     ` Arjan van de Ven
2023-03-01 20:32       ` Josh Triplett
2023-03-01 22:16         ` Paul E. McKenney
2023-03-01 22:25           ` Josh Triplett
2023-03-02  0:28             ` Paul E. McKenney
2023-03-02  1:05               ` Randy Dunlap
2023-03-02  1:37                 ` Paul E. McKenney [this message]
2023-02-28 21:57     ` David Woodhouse
2023-02-28 22:41       ` David Woodhouse
2023-02-28 22:48         ` Brian Gerst
2023-02-28 23:42           ` David Woodhouse
2023-03-01  0:02             ` H. Peter Anvin
2023-03-01  7:39               ` David Woodhouse
2023-03-01  0:03             ` Brian Gerst
2023-03-01 10:27       ` Thomas Gleixner
2023-02-28 22:09     ` Brian Gerst
2023-02-26 11:07 ` [PATCH v12 08/11] x86/smpboot: Remove initial_gs Usama Arif
2023-02-26 11:08 ` [PATCH v12 09/11] x86/smpboot: Support parallel startup of secondary CPUs Usama Arif
2023-02-26 11:08 ` [PATCH v12 10/11] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel Usama Arif
2023-02-26 11:08 ` [PATCH v12 11/11] x86/smpboot: Serialize topology updates for secondary bringup Usama Arif
2023-02-26 18:31 ` [PATCH v12 00/11] Parallel CPU bringup for x86_64 Oleksandr Natalenko
2023-02-26 20:59   ` [External] " Usama Arif
2023-02-27  6:13     ` David Woodhouse
2023-02-27  6:25       ` Usama Arif
2023-02-27  6:14     ` Usama Arif
2023-02-27 15:29       ` David Woodhouse
2023-02-27 16:32         ` Usama Arif
2023-02-27 21:39 ` Guilherme G. Piccoli
2023-02-28  9:07   ` David Woodhouse
2023-02-28 10:13   ` Paul Menzel
2023-02-28 12:04     ` Guilherme G. Piccoli

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=20230302013724.GO2948950@paulmck-ThinkPad-P17-Gen-1 \
    --to=paulmck@kernel.org \
    --cc=arjan@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dwmw2@infradead.org \
    --cc=dwmw@amazon.co.uk \
    --cc=fam.zheng@bytedance.com \
    --cc=hewenliang4@huawei.com \
    --cc=hpa@zytor.com \
    --cc=josh@joshtriplett.org \
    --cc=kim.phillips@amd.com \
    --cc=kvm@vger.kernel.org \
    --cc=liangma@liangbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mimoja@mimoja.de \
    --cc=mingo@redhat.com \
    --cc=oleksandr@natalenko.name \
    --cc=pbonzini@redhat.com \
    --cc=piotrgorski@cachyos.org \
    --cc=pmenzel@molgen.mpg.de \
    --cc=punit.agrawal@bytedance.com \
    --cc=rcu@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=seanjc@google.com \
    --cc=simon.evans@bytedance.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=usama.arif@bytedance.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

all inboxes | Powered by JetHome®