All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] rcu_barrier VS cpu_hotplug: make rcu_barrier can be called on CPU_DEAD
Date: Sat, 7 Mar 2009 09:20:19 -0800	[thread overview]
Message-ID: <20090307172019.GG10625@linux.vnet.ibm.com> (raw)
In-Reply-To: <49B2526C.5070503@cn.fujitsu.com>

On Sat, Mar 07, 2009 at 06:54:36PM +0800, Lai Jiangshan wrote:
> 
> cpu hotplug notifier_call for CPU_DEAD are very complex, some notifier_call
> may call rcu_barrier(), if this notifier_call is called earlier than
> rcu_cpu_notify(), it is buggy.
> 
> _cpu_down() -->
> 	some_cpu_hotplug_notify() -->
> 		case CPU_DEAD: rcu_barrier()
> 	rcu_cpu_notify() -->
> 		rcu_offline_cpu()
> 
> When rcu_barrier() is called, rcu_barrier() does not know how to finish
> his work: rcu callbacks in dead cpu have not been migrated to online cpu,
> so rcu_barrier() cannot wait for these rcu callbacks, but rcu_barrier()
> should wait for all queued rcu callbacks.

Hmmm...

What should happen in this case is that rcu_offline_cpu() moves the
rcu_barrier() callback to a surviving CPU, and then the rcu_barrier()
should complete just fine at the end of a subsequent grace period.
In fact, this is why it is so important to keep RCU callbacks ordered
through CPU-hotplug operations.

Or do you have a test that demonstrates a failure in this case?
If so, please share!

						Thanx, Paul

> This patch ensure that rcu_cpu_notify() is called earlier than 
> any other notifier_call.
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> index b86fa2f..f7ae5a3 100644
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -238,6 +238,12 @@ static inline int notifier_to_errno(int ret)
>  #define CPU_DYING_FROZEN	(CPU_DYING | CPU_TASKS_FROZEN)
>  #define CPU_STARTING_FROZEN	(CPU_STARTING | CPU_TASKS_FROZEN)
> 
> +/* Register at highest priority so that task migration (migrate_all_tasks)
> + * and rcu callbacks migration happens before everything else.
> + */
> +#define CPUHOTPLUG_TASK_MIGRATE_PRIORITY	10
> +#define CPUHOTPLUG_RCU_MIGRATE_PRIORITY		11
> +
>  /* Hibernation and suspend events */
>  #define PM_HIBERNATION_PREPARE	0x0001 /* Going to hibernate */
>  #define PM_POST_HIBERNATION	0x0002 /* Hibernation finished */
> diff --git a/kernel/rcuclassic.c b/kernel/rcuclassic.c
> index 654c640..040b894 100644
> --- a/kernel/rcuclassic.c
> +++ b/kernel/rcuclassic.c
> @@ -764,6 +764,7 @@ static int __cpuinit rcu_cpu_notify(struct notifier_block *self,
> 
>  static struct notifier_block __cpuinitdata rcu_nb = {
>  	.notifier_call	= rcu_cpu_notify,
> +	.priority	= CPUHOTPLUG_RCU_MIGRATE_PRIORITY,
>  };
> 
>  /*
> diff --git a/kernel/rcupreempt.c b/kernel/rcupreempt.c
> index 5d59e85..8ffcbf0 100644
> --- a/kernel/rcupreempt.c
> +++ b/kernel/rcupreempt.c
> @@ -1407,6 +1407,7 @@ static int __cpuinit rcu_cpu_notify(struct notifier_block *self,
> 
>  static struct notifier_block __cpuinitdata rcu_nb = {
>  	.notifier_call = rcu_cpu_notify,
> +	.priority = CPUHOTPLUG_RCU_MIGRATE_PRIORITY,
>  };
> 
>  void __init __rcu_init(void)
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index 97ce315..fcef06c 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1503,6 +1503,7 @@ do { \
> 
>  static struct notifier_block __cpuinitdata rcu_nb = {
>  	.notifier_call	= rcu_cpu_notify,
> +	.priority	= CPUHOTPLUG_RCU_MIGRATE_PRIORITY,
>  };
> 
>  void __init __rcu_init(void)
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 8e2558c..56f225e 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -6740,12 +6740,9 @@ migration_call(struct notifier_block *nfb, unsigned long action, void *hcpu)
>  	return NOTIFY_OK;
>  }
> 
> -/* Register at highest priority so that task migration (migrate_all_tasks)
> - * happens before everything else.
> - */
>  static struct notifier_block __cpuinitdata migration_notifier = {
>  	.notifier_call = migration_call,
> -	.priority = 10
> +	.priority = CPUHOTPLUG_TASK_MIGRATE_PRIORITY,
>  };
> 
>  static int __init migration_init(void)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

  reply	other threads:[~2009-03-07 17:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-07 10:54 [PATCH] rcu_barrier VS cpu_hotplug: make rcu_barrier can be called on CPU_DEAD Lai Jiangshan
2009-03-07 17:20 ` Paul E. McKenney [this message]
2009-03-08  3:05   ` Lai Jiangshan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090307172019.GG10625@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.