| 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: Fri, 25 Jul 2014 20:10:57 -0400
From: Pranith Kumar <bobby.prani@...il.com>
To: paulmck@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org
CC: mingo@...nel.org, laijs@...fujitsu.com, dipankar@...ibm.com,
akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
josh@...htriplett.org, tglx@...utronix.de, peterz@...radead.org,
rostedt@...dmis.org, dhowells@...hat.com, edumazet@...gle.com,
dvhart@...ux.intel.com, fweisbec@...il.com, oleg@...hat.com,
sasha.levin@...cle.com
Subject: Re: [PATCH RFC tip/core/rcu] Fix attempt to avoid offloading callbacks
unless requested
On 07/25/2014 07:36 PM, Paul E. McKenney wrote:
> [ Note: This applies on top of commit 187497fa5e9e (rcu: Allow for NULL
> tick_nohz_full_mask when nohz_full= missing) in -tip
> or -rcu. To make this work on top of rcu/next, move the
> call to rcu_organize_nocb_kthreads(rsp) to the end of the
> for_each_rcu_flavor(rsp) loop in rcu_init_nohz(). ]
>
> Commit b58cc46c5f6b (rcu: Don't offload callbacks unless specifically
> requested) failed to adjust the callback lists of the CPUs that are
> known to be no-CBs CPUs only because they are also nohz_full= CPUs.
> This failure can result in callbacks that are posted during early boot
> getting stranded on nxtlist for CPUs whose no-CBs property becomes
> apparent late, and there can also be spurious warnings about offline
> CPUs posting callbacks.
>
> This commit fixes these problems by adding an early-boot rcu_init_nohz()
> that properly initializes the no-CBs CPUs.
>
> Note that kernels built with CONFIG_RCU_NOCB_CPU_ALL=y or with
> CONFIG_RCU_NOCB_CPU=n do not exhibit this bug. Neither do kernels
> booted without the nohz_full= boot parameter.
>
> Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
Please find two points below.
<snip>
> #ifdef CONFIG_TREE_PREEMPT_RCU
> @@ -2451,6 +2424,66 @@ static void do_nocb_deferred_wakeup(struct rcu_data *rdp)
> trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu, TPS("DeferredWakeEmpty"));
> }
>
> +void rcu_init_nohz(void)
> +{
> + int cpu;
> + bool need_rcu_nocb_mask = true;
> + struct rcu_state *rsp;
> +
> +#ifdef CONFIG_RCU_NOCB_CPU_NONE
> + need_rcu_nocb_mask = false;
> +#endif /* #ifndef CONFIG_RCU_NOCB_CPU_NONE */
> +
> +#if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL)
> + if (tick_nohz_full_running && cpumask_weight(tick_nohz_full_mask))
> + need_rcu_nocb_mask = true;
> +#endif /* #if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL) */
> +
> + if (!have_rcu_nocb_mask && need_rcu_nocb_mask) {
> + zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL);
Please check the return value unless you want to increase my commit count ;)
>
> + have_rcu_nocb_mask = true;
> + }
> + if (!have_rcu_nocb_mask)
> + return;
> +
> +#ifdef CONFIG_RCU_NOCB_CPU_ZERO
> + pr_info("\tOffload RCU callbacks from CPU 0\n");
> + cpumask_set_cpu(0, rcu_nocb_mask);
> +#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ZERO */
> +#ifdef CONFIG_RCU_NOCB_CPU_ALL
> + pr_info("\tOffload RCU callbacks from all CPUs\n");
> + cpumask_copy(rcu_nocb_mask, cpu_possible_mask);
> +#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ALL */
> +#if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL)
> + cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
> +#endif /* #if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL) */
I understand that if CONFIG_NO_HZ_FULL_ALL is set then CONFIG_NOCB_CPU_ALL
will also be set and there is no need for this cpumask_or().
Is there any reason for the coupling between CONFIG_NO_HZ_FULL_ALL
and CONFIG_NOCB_CPU_ALL?
I ask because a user can override CONFIG_NO_HZ_FULL_ALL=y at boot time
using the nohz_full= boot time parameter. In this case even if a user marks
CPU 0 as the only nohz_full cpu, we will offload call backs from all CPUs.
Is this behavior what you have in mind?
--
Pranith
>
> +
> + if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
> + pr_info("\tNote: kernel parameter 'rcu_nocbs=' contains nonexistent CPUs.\n");
> + cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> + rcu_nocb_mask);
> + }
> + cpulist_scnprintf(nocb_buf, sizeof(nocb_buf), rcu_nocb_mask);
> + pr_info("\tOffload RCU callbacks from CPUs: %s.\n", nocb_buf);
> + if (rcu_nocb_poll)
> + pr_info("\tPoll for callbacks from no-CBs CPUs.\n");
> +
> + for_each_rcu_flavor(rsp) {
> + for_each_cpu(cpu, rcu_nocb_mask) {
> + struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
> +
> + /*
> + * If there are early callbacks, they will need
> + * to be moved to the nocb lists.
> + */
> + WARN_ON_ONCE(rdp->nxttail[RCU_NEXT_TAIL] !=
> + &rdp->nxtlist &&
> + rdp->nxttail[RCU_NEXT_TAIL] != NULL);
> + init_nocb_callback_list(rdp);
> + }
> + }
> +}
> +
> /* Initialize per-rcu_data variables for no-CBs CPUs. */
> static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
> {
> @@ -2479,10 +2512,6 @@ static void __init rcu_spawn_nocb_kthreads(struct rcu_state *rsp)
>
> if (rcu_nocb_mask == NULL)
> return;
> -#if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL)
> - if (tick_nohz_full_running)
> - cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
> -#endif /* #if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL) */
> if (ls == -1) {
> ls = int_sqrt(nr_cpu_ids);
> rcu_nocb_leader_stride = ls;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists