| 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
| ||
|
Message-ID: <20090319040503.GA7117@linux.vnet.ibm.com>
Date: Wed, 18 Mar 2009 21:05:03 -0700
From: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To: Lai Jiangshan <laijs@...fujitsu.com>
Cc: Ingo Molnar <mingo@...e.hu>, Peter Zijlstra <peterz@...radead.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rcu_barrier VS cpu_hotplug: Ensure callbacks in dead
cpu are migrated to online cpu
On Thu, Mar 19, 2009 at 11:06:39AM +0800, Lai Jiangshan wrote:
> Ingo Molnar wrote:
> > * Lai Jiangshan <laijs@...fujitsu.com> wrote:
> >
> >> [RFC]
> >> I don't like this patch, but I thought for some days and I can't
> >> thought out a better one.
> >
> > Interesting find. Found via code review or via testing? If via
> > testing, what is the symptom of the bug when it hits - did you
> > see CPU hotplug stress-tests hanging? Crashing too perhaps? How
> > frequently did it occur?
>
> I found this bug when I tested the draft version of kfree_rcu(V3).
>
> I noticed kfree_rcu_cpu_notify() is called earlier than
> rcu_cpu_notify(). This means rcu_barrier() is called earlier than
> RCU callbacks migration, it should lockup as expectation. But actually,
> this lockup can not occurred, I tried to explore it, and I found that
> rcu_barrier() does not handle cpu_hotplug. It includes two bugs.
>
> kfree_rcu(V3) (V4 is available too, it will be sent soon):
> http://lkml.org/lkml/2009/3/6/156
>
> The V1 fix of this bug:
> http://lkml.org/lkml/2009/3/7/38
>
> The fix of the other bug: (it changed the scheduler's code too)
> http://lkml.org/lkml/2009/3/7/39
>
> Subject: [PATCH] rcu_barrier VS cpu_hotplug: Ensure callbacks in dead cpu are migrated to online cpu (V2)
>
> cpu hotplug may be happened asynchronously, some rcu callbacks are maybe
> still in dead cpu, rcu_barrier() also needs to wait for these rcu callbacks
> to complete, so we must ensure callbacks in dead cpu are migrated to
> online cpu.
Good stuff, Lai!!! Simpler than any of the approaches that I was
considering, and, better yet, independent of the underlying RCU
implementation!!!
I was initially worried that wake_up() might wake only one of two
possible wait_event()s, namely rcu_barrier() and the CPU_POST_DEAD code,
but the fact that wait_event() clears WQ_FLAG_EXCLUSIVE avoids that issue.
I was also worried about the fact that different RCU implementations have
different mappings of call_rcu(), call_rcu_bh(), and call_rcu_sched(), but
this is OK as well because we just get an extra (harmless) callback in the
case that they map together (for example, Classic RCU has call_rcu_sched()
mapping to call_rcu()).
Overlap of CPU-hotplug operations is prevented by cpu_add_remove_lock,
and any stray callbacks that arrive (for example, from irq handlers
running on the dying CPU) either are ahead of the CPU_DYING callbacks on
the one hand (and thus accounted for), or happened after the rcu_barrier()
started on the other (and thus don't need to be accounted for).
So...
Reviewed-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com>
> ---
> diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c
> index cae8a05..2c7b845 100644
> --- a/kernel/rcupdate.c
> +++ b/kernel/rcupdate.c
> @@ -122,6 +122,8 @@ static void rcu_barrier_func(void *type)
> }
> }
>
> +static inline void wait_migrated_callbacks(void);
> +
> /*
> * Orchestrate the specified type of RCU barrier, waiting for all
> * RCU callbacks of the specified type to complete.
> @@ -147,6 +149,7 @@ static void _rcu_barrier(enum rcu_barrier type)
> complete(&rcu_barrier_completion);
> wait_for_completion(&rcu_barrier_completion);
> mutex_unlock(&rcu_barrier_mutex);
> + wait_migrated_callbacks();
> }
>
> /**
> @@ -176,9 +179,50 @@ void rcu_barrier_sched(void)
> }
> EXPORT_SYMBOL_GPL(rcu_barrier_sched);
>
> +static atomic_t rcu_migrate_type_count = ATOMIC_INIT(0);
> +static struct rcu_head rcu_migrate_head[3];
> +static DECLARE_WAIT_QUEUE_HEAD(rcu_migrate_wq);
> +
> +static void rcu_migrate_callback(struct rcu_head *notused)
> +{
> + if (atomic_dec_and_test(&rcu_migrate_type_count))
> + wake_up(&rcu_migrate_wq);
> +}
> +
> +static inline void wait_migrated_callbacks(void)
> +{
> + wait_event(rcu_migrate_wq, !atomic_read(&rcu_migrate_type_count));
> +}
> +
> +static int __cpuinit rcu_barrier_cpu_hotplug(struct notifier_block *self,
> + unsigned long action, void *hcpu)
> +{
> + if (action == CPU_DYING) {
> + /*
> + * preempt_disable() in on_each_cpu() prevents stop_machine(),
> + * so when "on_each_cpu(rcu_barrier_func, (void *)type, 1);"
> + * returns, all online cpus have queued rcu_barrier_func(),
> + * and the dead cpu(if it exist) queues rcu_migrate_callback()s.
> + *
> + * These callbacks ensure _rcu_barrier() waits for all
> + * RCU callbacks of the specified type to complete.
> + */
> + atomic_set(&rcu_migrate_type_count, 3);
> + call_rcu_bh(rcu_migrate_head, rcu_migrate_callback);
> + call_rcu_sched(rcu_migrate_head + 1, rcu_migrate_callback);
> + call_rcu(rcu_migrate_head + 2, rcu_migrate_callback);
> + } else if (action == CPU_POST_DEAD) {
> + /* rcu_migrate_head is protected by cpu_add_remove_lock */
> + wait_migrated_callbacks();
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> void __init rcu_init(void)
> {
> __rcu_init();
> + hotcpu_notifier(rcu_barrier_cpu_hotplug, 0);
> }
>
> void rcu_scheduler_starting(void)
>
--
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