| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Jul 2022 15:53:06 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Joel Fernandes <joel@...lfernandes.org>
Cc: Neeraj Upadhyay <quic_neeraju@...cinc.com>, rcu@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel-team@...com,
rostedt@...dmis.org, Kalesh Singh <kaleshsingh@...gle.com>,
Uladzislau Rezki <urezki@...il.com>,
kernel test robot <lkp@...el.com>
Subject: Re: [PATCH rcu 4/7] rcu/nocb: Add an option to offload all CPUs on
boot
On Tue, Jul 19, 2022 at 06:42:00PM -0400, Joel Fernandes wrote:
>
>
> On 7/19/2022 2:12 PM, Paul E. McKenney wrote:
> > On Tue, Jul 19, 2022 at 03:04:07PM +0530, Neeraj Upadhyay wrote:
> >>
> >>
> >> On 6/21/2022 4:15 AM, Paul E. McKenney wrote:
> >>> From: Joel Fernandes <joel@...lfernandes.org>
> >>>
> >>> Systems built with CONFIG_RCU_NOCB_CPU=y but booted without either
> >>> the rcu_nocbs= or rcu_nohz_full= kernel-boot parameters will not have
> >>> callback offloading on any of the CPUs, nor can any of the CPUs be
> >>> switched to enable callback offloading at runtime. Although this is
> >>> intentional, it would be nice to have a way to offload all the CPUs
> >>> without having to make random bootloaders specify either the rcu_nocbs=
> >>> or the rcu_nohz_full= kernel-boot parameters.
> >>>
> >>> This commit therefore provides a new CONFIG_RCU_NOCB_CPU_DEFAULT_ALL
> >>> Kconfig option that switches the default so as to offload callback
> >>> processing on all of the CPUs. This default can still be overridden
> >>> using the rcu_nocbs= and rcu_nohz_full= kernel-boot parameters.
> >>>
> >>> Reviewed-by: Kalesh Singh <kaleshsingh@...gle.com>
> >>> Reviewed-by: Uladzislau Rezki <urezki@...il.com>
> >>> (In v4.1, fixed issues with CONFIG maze reported by kernel test robot).
> >>> Reported-by: kernel test robot <lkp@...el.com>
> >>> Signed-off-by: Joel Fernandes <joel@...lfernandes.org>
> >>> Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
> >>> ---
> >>
> >>
> >> Reviewed-by: Neeraj Upadhyay <quic_neeraju@...cinc.com>
> >>
> >> One query on cpumask_setall() below
> >>
> >>> Documentation/admin-guide/kernel-parameters.txt | 6 ++++++
> >>> kernel/rcu/Kconfig | 13 +++++++++++++
> >>> kernel/rcu/tree_nocb.h | 15 ++++++++++++++-
> >>> 3 files changed, 33 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >>> index 2522b11e593f2..34605c275294c 100644
> >>> --- a/Documentation/admin-guide/kernel-parameters.txt
> >>> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >>> @@ -3659,6 +3659,9 @@
> >>> just as if they had also been called out in the
> >>> rcu_nocbs= boot parameter.
> >>> + Note that this argument takes precedence over
> >>> + the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> >>> +
> >>> noiotrap [SH] Disables trapped I/O port accesses.
> >>> noirqdebug [X86-32] Disables the code which attempts to detect and
> >>> @@ -4557,6 +4560,9 @@
> >>> no-callback mode from boot but the mode may be
> >>> toggled at runtime via cpusets.
> >>> + Note that this argument takes precedence over
> >>> + the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> >>> +
> >>> rcu_nocb_poll [KNL]
> >>> Rather than requiring that offloaded CPUs
> >>> (specified by rcu_nocbs= above) explicitly
> >>> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> >>> index 1c630e573548d..27aab870ae4cf 100644
> >>> --- a/kernel/rcu/Kconfig
> >>> +++ b/kernel/rcu/Kconfig
> >>> @@ -262,6 +262,19 @@ config RCU_NOCB_CPU
> >>> Say Y here if you need reduced OS jitter, despite added overhead.
> >>> Say N here if you are unsure.
> >>> +config RCU_NOCB_CPU_DEFAULT_ALL
> >>> + bool "Offload RCU callback processing from all CPUs by default"
> >>> + depends on RCU_NOCB_CPU
> >>> + default n
> >>> + help
> >>> + Use this option to offload callback processing from all CPUs
> >>> + by default, in the absence of the rcu_nocbs or nohz_full boot
> >>> + parameter. This also avoids the need to use any boot parameters
> >>> + to achieve the effect of offloading all CPUs on boot.
> >>> +
> >>> + Say Y here if you want offload all CPUs by default on boot.
> >>> + Say N here if you are unsure.
> >>> +
> >>> config TASKS_TRACE_RCU_READ_MB
> >>> bool "Tasks Trace RCU readers use memory barriers in user and idle"
> >>> depends on RCU_EXPERT && TASKS_TRACE_RCU
> >>> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> >>> index 4cf9a29bba79d..60cc92cc66552 100644
> >>> --- a/kernel/rcu/tree_nocb.h
> >>> +++ b/kernel/rcu/tree_nocb.h
> >>> @@ -1197,11 +1197,21 @@ void __init rcu_init_nohz(void)
> >>> {
> >>> int cpu;
> >>> bool need_rcu_nocb_mask = false;
> >>> + bool offload_all = false;
> >>> struct rcu_data *rdp;
> >>> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> >>> + if (!rcu_state.nocb_is_setup) {
> >>> + need_rcu_nocb_mask = true;
> >>> + offload_all = true;
> >>> + }
> >>> +#endif /* #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) */
> >>> +
> >>> #if defined(CONFIG_NO_HZ_FULL)
> >>> - if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> >>> + if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
> >>> need_rcu_nocb_mask = true;
> >>> + offload_all = false; /* NO_HZ_FULL has its own mask. */
> >>> + }
> >>> #endif /* #if defined(CONFIG_NO_HZ_FULL) */
> >>> if (need_rcu_nocb_mask) {
> >>> @@ -1222,6 +1232,9 @@ void __init rcu_init_nohz(void)
> >>> cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
> >>> #endif /* #if defined(CONFIG_NO_HZ_FULL) */
> >>> + if (offload_all)
> >>> + cpumask_setall(rcu_nocb_mask);
> >>
> >> Do we need to do a cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> >> rcu_nocb_mask) after setting all cpus in rcu_nocb_mask (cpumask_subset()
> >> check below takes care of it though)?
> >
> > Without that cpumask_and(), systems with sparse CPU numbering schemes
> > (for example, 0, 4, 8, 12, ...) will get a pr_info(), and as you noted,
> > the needed cpumask_and().
> >
> > I am inclined to see a complaint before we change this. And perhaps if
> > this is to change, the change should be in cpumask_setall() rather than
> > in rcu_init_nohz(). But that is an argument for later, if at all. ;-)
> >
> >>> +
> >>> if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
>
> We could also suppress the pr_info() by making it conditional.
>
> like:
>
> if (!CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) {
> pr_info(...);
> }
>
> In other words, we could make the cpumask_and() as expected/normal on
> systems with sparse CPU numbering schemes. Would that work?
That would be a good within-RCU workaround if we get an urgent complaint,
but if this requires a change, shouldn't cpumask_setall() refrain from
setting bits for non-existent CPUs? It does refrain from setting any
bits beyond the largest-numbered CPU.
But perhaps there is an early boot reason why cpumask_setall() cannot
do this?
Either way, we are just doing a pr_info(), not a WARN_ON() or similar,
so the current state is probably fine.
Thanx, Paul
> Thanks,
>
> - Joel
>
>
> >>> pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
>
>
>
> >>> cpumask_and(rcu_nocb_mask, cpu_possible_mask,
Powered by blists - more mailing lists