* [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default [not found] ` <20161031181543.GN3716@linux.vnet.ibm.com> @ 2016-11-02 16:30 ` Sebastian Andrzej Siewior 2016-11-03 16:22 ` Paul E. McKenney 0 siblings, 1 reply; 10+ messages in thread From: Sebastian Andrzej Siewior @ 2016-11-02 16:30 UTC (permalink / raw) To: Paul E. McKenney Cc: Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, linux-kernel RCU_EXPEDITE_BOOT should speed up the boot process by enforcing synchronize_rcu_expedited() instead of synchronize_rcu() during the boot process. There should be no reason why one does not want this and there is no need worry about real time latency at this point. Therefore make it default. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- Checked on two boxes and synchronize_rcu() was invoked four times before it reached rcu_end_inkernel_boot() init/Kconfig | 13 ------------- kernel/rcu/update.c | 6 ++---- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index b6c9166d878a..fe51bd3bbc61 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -771,19 +771,6 @@ config RCU_NOCB_CPU_ALL endchoice -config RCU_EXPEDITE_BOOT - bool - default n - help - This option enables expedited grace periods at boot time, - as if rcu_expedite_gp() had been invoked early in boot. - The corresponding rcu_unexpedite_gp() is invoked from - rcu_end_inkernel_boot(), which is intended to be invoked - at the end of the kernel-only boot sequence, just before - init is exec'ed. - - Accept the default if unsure. - endmenu # "RCU Subsystem" config BUILD_BIN2C diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c index b40d3468ba4e..419ca811bda9 100644 --- a/kernel/rcu/update.c +++ b/kernel/rcu/update.c @@ -130,8 +130,7 @@ bool rcu_gp_is_normal(void) } EXPORT_SYMBOL_GPL(rcu_gp_is_normal); -static atomic_t rcu_expedited_nesting = - ATOMIC_INIT(IS_ENABLED(CONFIG_RCU_EXPEDITE_BOOT) ? 1 : 0); +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); /* * Should normal grace-period primitives be expedited? Intended for @@ -179,8 +178,7 @@ EXPORT_SYMBOL_GPL(rcu_unexpedite_gp); */ void rcu_end_inkernel_boot(void) { - if (IS_ENABLED(CONFIG_RCU_EXPEDITE_BOOT)) - rcu_unexpedite_gp(); + rcu_unexpedite_gp(); if (rcu_normal_after_boot) WRITE_ONCE(rcu_normal, 1); } -- 2.10.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-02 16:30 ` [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default Sebastian Andrzej Siewior @ 2016-11-03 16:22 ` Paul E. McKenney 2016-11-03 16:33 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 10+ messages in thread From: Paul E. McKenney @ 2016-11-03 16:22 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On Wed, Nov 02, 2016 at 05:30:02PM +0100, Sebastian Andrzej Siewior wrote: > RCU_EXPEDITE_BOOT should speed up the boot process by enforcing > synchronize_rcu_expedited() instead of synchronize_rcu() during the boot > process. There should be no reason why one does not want this and there > is no need worry about real time latency at this point. > Therefore make it default. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Well, it has been awhile since I removed a Kconfig parameter. So why could this be a bad thing? 1. Very large systems might see scalability issues with unconditional expediting at boot. But if we don't try it, we won't know. 2. People bringing up new hardware might not want quite so many IPIs. But they can just set rcu_normal to prevent that. I am therefore queuing it for testiong and review. ;-) Thanx, Paul > --- > Checked on two boxes and synchronize_rcu() was invoked four times before > it reached rcu_end_inkernel_boot() > > init/Kconfig | 13 ------------- > kernel/rcu/update.c | 6 ++---- > 2 files changed, 2 insertions(+), 17 deletions(-) > > diff --git a/init/Kconfig b/init/Kconfig > index b6c9166d878a..fe51bd3bbc61 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -771,19 +771,6 @@ config RCU_NOCB_CPU_ALL > > endchoice > > -config RCU_EXPEDITE_BOOT > - bool > - default n > - help > - This option enables expedited grace periods at boot time, > - as if rcu_expedite_gp() had been invoked early in boot. > - The corresponding rcu_unexpedite_gp() is invoked from > - rcu_end_inkernel_boot(), which is intended to be invoked > - at the end of the kernel-only boot sequence, just before > - init is exec'ed. > - > - Accept the default if unsure. > - > endmenu # "RCU Subsystem" > > config BUILD_BIN2C > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c > index b40d3468ba4e..419ca811bda9 100644 > --- a/kernel/rcu/update.c > +++ b/kernel/rcu/update.c > @@ -130,8 +130,7 @@ bool rcu_gp_is_normal(void) > } > EXPORT_SYMBOL_GPL(rcu_gp_is_normal); > > -static atomic_t rcu_expedited_nesting = > - ATOMIC_INIT(IS_ENABLED(CONFIG_RCU_EXPEDITE_BOOT) ? 1 : 0); > +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > > /* > * Should normal grace-period primitives be expedited? Intended for > @@ -179,8 +178,7 @@ EXPORT_SYMBOL_GPL(rcu_unexpedite_gp); > */ > void rcu_end_inkernel_boot(void) > { > - if (IS_ENABLED(CONFIG_RCU_EXPEDITE_BOOT)) > - rcu_unexpedite_gp(); > + rcu_unexpedite_gp(); > if (rcu_normal_after_boot) > WRITE_ONCE(rcu_normal, 1); > } > -- > 2.10.2 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-03 16:22 ` Paul E. McKenney @ 2016-11-03 16:33 ` Sebastian Andrzej Siewior 2016-11-03 16:59 ` Paul E. McKenney 0 siblings, 1 reply; 10+ messages in thread From: Sebastian Andrzej Siewior @ 2016-11-03 16:33 UTC (permalink / raw) To: Paul E. McKenney Cc: Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On 2016-11-03 09:22:28 [-0700], Paul E. McKenney wrote: > On Wed, Nov 02, 2016 at 05:30:02PM +0100, Sebastian Andrzej Siewior wrote: > > RCU_EXPEDITE_BOOT should speed up the boot process by enforcing > > synchronize_rcu_expedited() instead of synchronize_rcu() during the boot > > process. There should be no reason why one does not want this and there > > is no need worry about real time latency at this point. > > Therefore make it default. > > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > Well, it has been awhile since I removed a Kconfig parameter. > > So why could this be a bad thing? > > 1. Very large systems might see scalability issues with unconditional > expediting at boot. But if we don't try it, we won't know. You mean we would make the boot process slower for them instead of faster? > 2. People bringing up new hardware might not want quite so many > IPIs. But they can just set rcu_normal to prevent that. I wanted to make things simple and not complicated… > I am therefore queuing it for testiong and review. ;-) Okay thanks. > Thanx, Paul Sebastian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-03 16:33 ` Sebastian Andrzej Siewior @ 2016-11-03 16:59 ` Paul E. McKenney 2016-11-07 17:19 ` Steven Rostedt 0 siblings, 1 reply; 10+ messages in thread From: Paul E. McKenney @ 2016-11-03 16:59 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On Thu, Nov 03, 2016 at 05:33:27PM +0100, Sebastian Andrzej Siewior wrote: > On 2016-11-03 09:22:28 [-0700], Paul E. McKenney wrote: > > On Wed, Nov 02, 2016 at 05:30:02PM +0100, Sebastian Andrzej Siewior wrote: > > > RCU_EXPEDITE_BOOT should speed up the boot process by enforcing > > > synchronize_rcu_expedited() instead of synchronize_rcu() during the boot > > > process. There should be no reason why one does not want this and there > > > is no need worry about real time latency at this point. > > > Therefore make it default. > > > > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > > > Well, it has been awhile since I removed a Kconfig parameter. > > > > So why could this be a bad thing? > > > > 1. Very large systems might see scalability issues with unconditional > > expediting at boot. But if we don't try it, we won't know. > > You mean we would make the boot process slower for them instead of > faster? For really bit systems, quite possibly, where "really big" means many hundreds or (more likely) thousands of CPUs. But there are things that I can do to fix this when and if. > > 2. People bringing up new hardware might not want quite so many > > IPIs. But they can just set rcu_normal to prevent that. > > I wanted to make things simple and not complicated… I know that feeling. ;-) > > I am therefore queuing it for testiong and review. ;-) > > Okay thanks. Thanx, Paul > Sebastian > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-03 16:59 ` Paul E. McKenney @ 2016-11-07 17:19 ` Steven Rostedt 2016-11-07 17:30 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 10+ messages in thread From: Steven Rostedt @ 2016-11-07 17:19 UTC (permalink / raw) To: Paul E. McKenney Cc: Sebastian Andrzej Siewior, Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On Thu, 3 Nov 2016 09:59:31 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote: > On Thu, Nov 03, 2016 at 05:33:27PM +0100, Sebastian Andrzej Siewior wrote: > > On 2016-11-03 09:22:28 [-0700], Paul E. McKenney wrote: > > > On Wed, Nov 02, 2016 at 05:30:02PM +0100, Sebastian Andrzej Siewior wrote: > > > > RCU_EXPEDITE_BOOT should speed up the boot process by enforcing > > > > synchronize_rcu_expedited() instead of synchronize_rcu() during the boot > > > > process. There should be no reason why one does not want this and there > > > > is no need worry about real time latency at this point. > > > > Therefore make it default. > > > > > > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > > > > > Well, it has been awhile since I removed a Kconfig parameter. > > > > > > So why could this be a bad thing? > > > > > > 1. Very large systems might see scalability issues with unconditional > > > expediting at boot. But if we don't try it, we won't know. > > > > You mean we would make the boot process slower for them instead of > > faster? > > For really bit systems, quite possibly, where "really big" means > many hundreds or (more likely) thousands of CPUs. > > But there are things that I can do to fix this when and if. > > > > 2. People bringing up new hardware might not want quite so many > > > IPIs. But they can just set rcu_normal to prevent that. > > > > I wanted to make things simple and not complicated… > > I know that feeling. ;-) > I agree, but if this creates a boot time regression in large machines, it may not be warranted. I know Linus usually doesn't like options with default y, but this may be one of those exceptions. Perhaps we should make it on by default and say in the config "if you have a machine with 100s or 1000s of CPUs, you may want to disable this". -- Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-07 17:19 ` Steven Rostedt @ 2016-11-07 17:30 ` Sebastian Andrzej Siewior 2016-11-07 17:35 ` Josh Triplett 0 siblings, 1 reply; 10+ messages in thread From: Sebastian Andrzej Siewior @ 2016-11-07 17:30 UTC (permalink / raw) To: Steven Rostedt Cc: Paul E. McKenney, Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On 2016-11-07 12:19:39 [-0500], Steven Rostedt wrote: > I agree, but if this creates a boot time regression in large machines, > it may not be warranted. > > I know Linus usually doesn't like options with default y, but this may > be one of those exceptions. Perhaps we should make it on by default and > say in the config "if you have a machine with 100s or 1000s of CPUs, > you may want to disable this". The default could change if we know where the limit is. I have access to a box with approx 140 CPUs so I could check there if it is already bad. But everything above that / in the 1000 range is a different story. > -- Steve Sebastian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-07 17:30 ` Sebastian Andrzej Siewior @ 2016-11-07 17:35 ` Josh Triplett 2016-11-07 18:05 ` Paul E. McKenney 0 siblings, 1 reply; 10+ messages in thread From: Josh Triplett @ 2016-11-07 17:35 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Steven Rostedt, Paul E. McKenney, Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On Mon, Nov 07, 2016 at 06:30:30PM +0100, Sebastian Andrzej Siewior wrote: > On 2016-11-07 12:19:39 [-0500], Steven Rostedt wrote: > > I agree, but if this creates a boot time regression in large machines, > > it may not be warranted. > > > > I know Linus usually doesn't like options with default y, but this may > > be one of those exceptions. Perhaps we should make it on by default and > > say in the config "if you have a machine with 100s or 1000s of CPUs, > > you may want to disable this". > > The default could change if we know where the limit is. I have access to > a box with approx 140 CPUs so I could check there if it is already bad. > But everything above that / in the 1000 range is a different story. Right; if we can characterize what machines it benefits and what machines it hurts, we can automatically detect and run the appropriate case with no configuration option needed. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-07 17:35 ` Josh Triplett @ 2016-11-07 18:05 ` Paul E. McKenney 2016-11-07 18:08 ` Josh Triplett 0 siblings, 1 reply; 10+ messages in thread From: Paul E. McKenney @ 2016-11-07 18:05 UTC (permalink / raw) To: Josh Triplett Cc: Sebastian Andrzej Siewior, Steven Rostedt, Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On Mon, Nov 07, 2016 at 09:35:46AM -0800, Josh Triplett wrote: > On Mon, Nov 07, 2016 at 06:30:30PM +0100, Sebastian Andrzej Siewior wrote: > > On 2016-11-07 12:19:39 [-0500], Steven Rostedt wrote: > > > I agree, but if this creates a boot time regression in large machines, > > > it may not be warranted. > > > > > > I know Linus usually doesn't like options with default y, but this may > > > be one of those exceptions. Perhaps we should make it on by default and > > > say in the config "if you have a machine with 100s or 1000s of CPUs, > > > you may want to disable this". > > > > The default could change if we know where the limit is. I have access to > > a box with approx 140 CPUs so I could check there if it is already bad. > > But everything above that / in the 1000 range is a different story. > > Right; if we can characterize what machines it benefits and what > machines it hurts, we can automatically detect and run the appropriate > case with no configuration option needed. I very much like this approach! Anyone have access to large systems on which this experiment could be carried out? In the absence of new data, I would just set the cutoff at 256 CPUs, as I have done in the past. Thanx, Paul ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-07 18:05 ` Paul E. McKenney @ 2016-11-07 18:08 ` Josh Triplett 2016-11-07 19:04 ` Paul E. McKenney 0 siblings, 1 reply; 10+ messages in thread From: Josh Triplett @ 2016-11-07 18:08 UTC (permalink / raw) To: Paul E. McKenney Cc: Sebastian Andrzej Siewior, Steven Rostedt, Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On Mon, Nov 07, 2016 at 10:05:13AM -0800, Paul E. McKenney wrote: > On Mon, Nov 07, 2016 at 09:35:46AM -0800, Josh Triplett wrote: > > On Mon, Nov 07, 2016 at 06:30:30PM +0100, Sebastian Andrzej Siewior wrote: > > > On 2016-11-07 12:19:39 [-0500], Steven Rostedt wrote: > > > > I agree, but if this creates a boot time regression in large machines, > > > > it may not be warranted. > > > > > > > > I know Linus usually doesn't like options with default y, but this may > > > > be one of those exceptions. Perhaps we should make it on by default and > > > > say in the config "if you have a machine with 100s or 1000s of CPUs, > > > > you may want to disable this". > > > > > > The default could change if we know where the limit is. I have access to > > > a box with approx 140 CPUs so I could check there if it is already bad. > > > But everything above that / in the 1000 range is a different story. > > > > Right; if we can characterize what machines it benefits and what > > machines it hurts, we can automatically detect and run the appropriate > > case with no configuration option needed. > > I very much like this approach! Anyone have access to large systems on > which this experiment could be carried out? In the absence of new data, > I would just set the cutoff at 256 CPUs, as I have done in the past. One potential issue here: the point where RCU_EXPEDITE_BOOT pessimizes likely depends on interconnect as much as CPUs. I'd guess that you may want to set the cutoff based on number of NUMA nodes, rather than number of CPUs. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default 2016-11-07 18:08 ` Josh Triplett @ 2016-11-07 19:04 ` Paul E. McKenney 0 siblings, 0 replies; 10+ messages in thread From: Paul E. McKenney @ 2016-11-07 19:04 UTC (permalink / raw) To: Josh Triplett Cc: Sebastian Andrzej Siewior, Steven Rostedt, Michael S. Tsirkin, Julia Cartwright, Luiz Capitulino, linux-rt-users, Mathieu Desnoyers, Lai Jiangshan, linux-kernel On Mon, Nov 07, 2016 at 10:08:32AM -0800, Josh Triplett wrote: > On Mon, Nov 07, 2016 at 10:05:13AM -0800, Paul E. McKenney wrote: > > On Mon, Nov 07, 2016 at 09:35:46AM -0800, Josh Triplett wrote: > > > On Mon, Nov 07, 2016 at 06:30:30PM +0100, Sebastian Andrzej Siewior wrote: > > > > On 2016-11-07 12:19:39 [-0500], Steven Rostedt wrote: > > > > > I agree, but if this creates a boot time regression in large machines, > > > > > it may not be warranted. > > > > > > > > > > I know Linus usually doesn't like options with default y, but this may > > > > > be one of those exceptions. Perhaps we should make it on by default and > > > > > say in the config "if you have a machine with 100s or 1000s of CPUs, > > > > > you may want to disable this". > > > > > > > > The default could change if we know where the limit is. I have access to > > > > a box with approx 140 CPUs so I could check there if it is already bad. > > > > But everything above that / in the 1000 range is a different story. > > > > > > Right; if we can characterize what machines it benefits and what > > > machines it hurts, we can automatically detect and run the appropriate > > > case with no configuration option needed. > > > > I very much like this approach! Anyone have access to large systems on > > which this experiment could be carried out? In the absence of new data, > > I would just set the cutoff at 256 CPUs, as I have done in the past. > > One potential issue here: the point where RCU_EXPEDITE_BOOT pessimizes > likely depends on interconnect as much as CPUs. I'd guess that you may > want to set the cutoff based on number of NUMA nodes, rather than number > of CPUs. Longer term, the solution would be a function that defined the cutoff point. If an architecture didn't define the function, they get the default cutoff of 256. If they do define the function, they get whatever they coded. Thanx, Paul ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-07 19:04 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20161012162114.GA9362@jcartwri.amer.corp.natinst.com> [not found] ` <20161012124956.3cb5f988@redhat.com> [not found] ` <20161012171553.GA18392@jcartwri.amer.corp.natinst.com> [not found] ` <20161012203223.GK29518@linux.vnet.ibm.com> [not found] ` <20161013191332-mutt-send-email-mst@kernel.org> [not found] ` <20161014092050.GW29518@linux.vnet.ibm.com> [not found] ` <20161016044420-mutt-send-email-mst@kernel.org> [not found] ` <20161016112846.GR29518@linux.vnet.ibm.com> [not found] ` <20161031173852.a3ji7hhgjis5l3u4@linutronix.de> [not found] ` <20161031181543.GN3716@linux.vnet.ibm.com> 2016-11-02 16:30 ` [PATCH] rcu: update: make RCU_EXPEDITE_BOOT default Sebastian Andrzej Siewior 2016-11-03 16:22 ` Paul E. McKenney 2016-11-03 16:33 ` Sebastian Andrzej Siewior 2016-11-03 16:59 ` Paul E. McKenney 2016-11-07 17:19 ` Steven Rostedt 2016-11-07 17:30 ` Sebastian Andrzej Siewior 2016-11-07 17:35 ` Josh Triplett 2016-11-07 18:05 ` Paul E. McKenney 2016-11-07 18:08 ` Josh Triplett 2016-11-07 19:04 ` Paul E. McKenney
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).