| Message ID | 20220817014253.1982-3-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rcu/nocb: Delete local variable 'need_rcu_nocb_mask' in rcu_init_nohz() | expand |
On Wed, Aug 17, 2022 at 09:42:53AM +0800, Zhen Lei wrote: > When CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or CONFIG_NO_HZ_FULL=y, additional > CPUs need to be added to 'rcu_nocb_mask'. But 'rcu_nocb_mask' may be not > available now, due to 'rcu_nocbs' is not specified. Check and initialize > 'rcu_nocb_mask' before using it. This code simplification strictly follows > this logic, compared with old implementations, unnecessary crossovers are > avoided and easy to understand. > > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> This looks like a nice simplification, but I will wait for your response on Patch 1/1 before trying it out. Thanx, Paul > --- > kernel/rcu/tree_nocb.h | 32 +++++++++----------------------- > 1 file changed, 9 insertions(+), 23 deletions(-) > > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h > index ff763e7dc53551f..3c59b12f4465af1 100644 > --- a/kernel/rcu/tree_nocb.h > +++ b/kernel/rcu/tree_nocb.h > @@ -1209,44 +1209,30 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload); > void __init rcu_init_nohz(void) > { > int cpu; > - bool need_rcu_nocb_mask = false; > - bool offload_all = false; > struct rcu_data *rdp; > + const struct cpumask *cpumask = NULL; > > #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) > - if (!cpumask_available(rcu_nocb_mask)) { > - 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)) { > - need_rcu_nocb_mask = true; > - offload_all = false; /* NO_HZ_FULL has its own mask. */ > - } > -#endif /* #if defined(CONFIG_NO_HZ_FULL) */ > + cpumask = cpu_possible_mask; > +#elif defined(CONFIG_NO_HZ_FULL) > + if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) > + cpumask = tick_nohz_full_mask; > +#endif > > - if (need_rcu_nocb_mask) { > + if (cpumask) { > if (!cpumask_available(rcu_nocb_mask)) { > if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) { > pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n"); > return; > } > } > + > + cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask); > } > > if (!cpumask_available(rcu_nocb_mask)) > return; > > -#if defined(CONFIG_NO_HZ_FULL) > - if (tick_nohz_full_running) > - 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); > - > if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) { > pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n"); > cpumask_and(rcu_nocb_mask, cpu_possible_mask, > -- > 2.25.1 >
On 2022/8/23 0:35, Paul E. McKenney wrote: > On Wed, Aug 17, 2022 at 09:42:53AM +0800, Zhen Lei wrote: >> When CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or CONFIG_NO_HZ_FULL=y, additional >> CPUs need to be added to 'rcu_nocb_mask'. But 'rcu_nocb_mask' may be not >> available now, due to 'rcu_nocbs' is not specified. Check and initialize >> 'rcu_nocb_mask' before using it. This code simplification strictly follows >> this logic, compared with old implementations, unnecessary crossovers are >> avoided and easy to understand. >> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > This looks like a nice simplification, but I will wait for your response > on Patch 1/1 before trying it out. How about I post v5 and just do this simplification? Patch 1/2 seems to require further discussion and in-depth analysis, which may take a long time. > > Thanx, Paul > >> --- >> kernel/rcu/tree_nocb.h | 32 +++++++++----------------------- >> 1 file changed, 9 insertions(+), 23 deletions(-) >> >> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h >> index ff763e7dc53551f..3c59b12f4465af1 100644 >> --- a/kernel/rcu/tree_nocb.h >> +++ b/kernel/rcu/tree_nocb.h >> @@ -1209,44 +1209,30 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload); >> void __init rcu_init_nohz(void) >> { >> int cpu; >> - bool need_rcu_nocb_mask = false; >> - bool offload_all = false; >> struct rcu_data *rdp; >> + const struct cpumask *cpumask = NULL; >> >> #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) >> - if (!cpumask_available(rcu_nocb_mask)) { >> - 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)) { >> - need_rcu_nocb_mask = true; >> - offload_all = false; /* NO_HZ_FULL has its own mask. */ >> - } >> -#endif /* #if defined(CONFIG_NO_HZ_FULL) */ >> + cpumask = cpu_possible_mask; >> +#elif defined(CONFIG_NO_HZ_FULL) >> + if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) >> + cpumask = tick_nohz_full_mask; >> +#endif >> >> - if (need_rcu_nocb_mask) { >> + if (cpumask) { >> if (!cpumask_available(rcu_nocb_mask)) { >> if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) { >> pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n"); >> return; >> } >> } >> + >> + cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask); >> } >> >> if (!cpumask_available(rcu_nocb_mask)) >> return; >> >> -#if defined(CONFIG_NO_HZ_FULL) >> - if (tick_nohz_full_running) >> - 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); >> - >> if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) { >> pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n"); >> cpumask_and(rcu_nocb_mask, cpu_possible_mask, >> -- >> 2.25.1 >> > . >
On Tue, Aug 23, 2022 at 08:13:00PM +0800, Leizhen (ThunderTown) wrote: > > > On 2022/8/23 0:35, Paul E. McKenney wrote: > > On Wed, Aug 17, 2022 at 09:42:53AM +0800, Zhen Lei wrote: > >> When CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or CONFIG_NO_HZ_FULL=y, additional > >> CPUs need to be added to 'rcu_nocb_mask'. But 'rcu_nocb_mask' may be not > >> available now, due to 'rcu_nocbs' is not specified. Check and initialize > >> 'rcu_nocb_mask' before using it. This code simplification strictly follows > >> this logic, compared with old implementations, unnecessary crossovers are > >> avoided and easy to understand. > >> > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > > > This looks like a nice simplification, but I will wait for your response > > on Patch 1/1 before trying it out. > > How about I post v5 and just do this simplification? Patch 1/2 seems to require > further discussion and in-depth analysis, which may take a long time. Excellent choice, thank you! Thanx, Paul > >> --- > >> kernel/rcu/tree_nocb.h | 32 +++++++++----------------------- > >> 1 file changed, 9 insertions(+), 23 deletions(-) > >> > >> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h > >> index ff763e7dc53551f..3c59b12f4465af1 100644 > >> --- a/kernel/rcu/tree_nocb.h > >> +++ b/kernel/rcu/tree_nocb.h > >> @@ -1209,44 +1209,30 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload); > >> void __init rcu_init_nohz(void) > >> { > >> int cpu; > >> - bool need_rcu_nocb_mask = false; > >> - bool offload_all = false; > >> struct rcu_data *rdp; > >> + const struct cpumask *cpumask = NULL; > >> > >> #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) > >> - if (!cpumask_available(rcu_nocb_mask)) { > >> - 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)) { > >> - need_rcu_nocb_mask = true; > >> - offload_all = false; /* NO_HZ_FULL has its own mask. */ > >> - } > >> -#endif /* #if defined(CONFIG_NO_HZ_FULL) */ > >> + cpumask = cpu_possible_mask; > >> +#elif defined(CONFIG_NO_HZ_FULL) > >> + if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) > >> + cpumask = tick_nohz_full_mask; > >> +#endif > >> > >> - if (need_rcu_nocb_mask) { > >> + if (cpumask) { > >> if (!cpumask_available(rcu_nocb_mask)) { > >> if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) { > >> pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n"); > >> return; > >> } > >> } > >> + > >> + cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask); > >> } > >> > >> if (!cpumask_available(rcu_nocb_mask)) > >> return; > >> > >> -#if defined(CONFIG_NO_HZ_FULL) > >> - if (tick_nohz_full_running) > >> - 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); > >> - > >> if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) { > >> pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n"); > >> cpumask_and(rcu_nocb_mask, cpu_possible_mask, > >> -- > >> 2.25.1 > >> > > . > > > > -- > Regards, > Zhen Lei
diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h index ff763e7dc53551f..3c59b12f4465af1 100644 --- a/kernel/rcu/tree_nocb.h +++ b/kernel/rcu/tree_nocb.h @@ -1209,44 +1209,30 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload); void __init rcu_init_nohz(void) { int cpu; - bool need_rcu_nocb_mask = false; - bool offload_all = false; struct rcu_data *rdp; + const struct cpumask *cpumask = NULL; #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) - if (!cpumask_available(rcu_nocb_mask)) { - 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)) { - need_rcu_nocb_mask = true; - offload_all = false; /* NO_HZ_FULL has its own mask. */ - } -#endif /* #if defined(CONFIG_NO_HZ_FULL) */ + cpumask = cpu_possible_mask; +#elif defined(CONFIG_NO_HZ_FULL) + if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) + cpumask = tick_nohz_full_mask; +#endif - if (need_rcu_nocb_mask) { + if (cpumask) { if (!cpumask_available(rcu_nocb_mask)) { if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) { pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n"); return; } } + + cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask); } if (!cpumask_available(rcu_nocb_mask)) return; -#if defined(CONFIG_NO_HZ_FULL) - if (tick_nohz_full_running) - 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); - if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) { pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n"); cpumask_and(rcu_nocb_mask, cpu_possible_mask,
When CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or CONFIG_NO_HZ_FULL=y, additional CPUs need to be added to 'rcu_nocb_mask'. But 'rcu_nocb_mask' may be not available now, due to 'rcu_nocbs' is not specified. Check and initialize 'rcu_nocb_mask' before using it. This code simplification strictly follows this logic, compared with old implementations, unnecessary crossovers are avoided and easy to understand. Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- kernel/rcu/tree_nocb.h | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-)