[RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

Hi,

This patchset implements the approach of invoking the CPU hotplug callbacks
(notifiers) in one order during CPU online and in the reverse order during CPU
offline. The rationale behind this is that services for a CPU are started in a
particular order (perhaps, with implicit dependencies between them) while
bringing up the CPU, and hence, it makes sense to tear down the services in
the opposite order, thereby honoring most of the dependencies automatically
(and also correctly). This is explained in more detail in Patch 6.

Patch 1 adds rcu primitives for traversing a linked list in reverse order.
Patch 2 converts the notifiers to a circular doubly linked list (in
preparation for the reverse invocation support).
Patch 3 adds helpers to invoke notifiers in the reverse order.
Patches 4 and 5 do the necessary prep work for moving over to the reverse
invocation model, by adjusting some of the callbacks/priorities as necessary.
Patch 6 does the final transition and invokes the notifiers in the reverse order
during CPU offline.

This patchset applies on top of:
1. Tejun's workqueue CPU hotplug patchset[1] merged with
2. Thomas Gleixner's park/unpark patchset with Paul McKenney's fixes which
   are available at [2].

References:
[1]. git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-wq-hotplug
[2]. git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git rcu/smp/hotplug

--
 Srivatsa S. Bhat (6):
      list, rcu: Introduce rcu version of reverse list traversal
      notifiers: Convert notifier chain to circular doubly linked-list
      notifiers: Add support for reverse invocation of notifier chains
      sched, cpuset: Prepare scheduler and cpuset CPU hotplug callbacks for reverse invocation
      sched, perf: Prepare migration and perf CPU hotplug callbacks for reverse invocation
      CPU hotplug: Invoke CPU offline notifiers in reverse order


  arch/mips/powertv/powertv_setup.c                |    2 
 arch/um/drivers/mconsole_kern.c                  |    2 
 arch/um/kernel/um_arch.c                         |    2 
 drivers/acpi/sleep.c                             |    2 
 drivers/char/ipmi/ipmi_msghandler.c              |    2 
 drivers/char/ipmi/ipmi_watchdog.c                |    4 
 drivers/firmware/dcdbas.c                        |    2 
 drivers/md/md.c                                  |    2 
 drivers/net/ethernet/intel/igb/igb_main.c        |    2 
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    |    2 
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c |    2 
 drivers/staging/vt6655/device_main.c             |    2 
 include/linux/cpu.h                              |   40 +++--
 include/linux/notifier.h                         |   27 ++-
 include/linux/rculist.h                          |   46 ++++++
 kernel/cpu.c                                     |   34 +++-
 kernel/debug/debug_core.c                        |    2 
 kernel/notifier.c                                |  179 ++++++++++++++++------
 kernel/sched/core.c                              |  140 ++++++++++-------
 kernel/trace/trace.c                             |    2 
 kernel/workqueue.c                               |   30 +---
 mm/page-writeback.c                              |    2 
 mm/vmstat.c                                      |    4 
 23 files changed, 357 insertions(+), 175 deletions(-)



Thanks,
Srivatsa S. Bhat
IBM Linux Technology Center

[RFC PATCH 1/6] list, rcu: Introduce rcu version of reverse list traversal

[Srivatsa S. Bhat]

Provide a helper to traverse an RCU-protected doubly linked list in the reverse
order. Also add a corresponding helper to delete entries from the linked list,
which takes care to see that we don't poison either of the pointers (->prev
and ->next), since it is legal to run an rcu list-traversal operation (in
either forward/reverse direction) concurrently.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/rculist.h |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index e0f0fab..6f22a13 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -118,6 +118,36 @@ static inline void list_del_rcu(struct list_head *entry)
 }
 
 /**
+ * list_del_bidir_rcu - deletes entry from list without re-initialization.
+ * Useful when one wants to perform bidirectional traversal of the list.
+ *
+ * @entry: the element to delete from the list.
+ *
+ * Note: list_empty() on entry does not return true after this,
+ * the entry is in an undefined state. It is useful for RCU based
+ * lockfree traversal.
+ *
+ * In particular, since we should be able to traverse this list in both
+ * directions, it means that we can not poison either of the pointers
+ * (->prev and ->next) because they may still be used for walking the list.
+ *
+ * The caller must take whatever precautions are necessary (such as holding
+ * appropriate locks) to avoid racing with another list-mutation primitive,
+ * such as list_del_bidir_rcu() or list_add_rcu(), running on this same list.
+ * However, it is perfectly legal to run concurrently with the _rcu
+ * list-traversal primitives, such as list_for_each_entry_rcu() or
+ * list_for_each_entry_reverse_rcu().
+ *
+ * Note that the caller is not permitted to immediately free the newly deleted
+ * entry.  Instead, either synchronize_rcu() or call_rcu() must be used to
+ * defer freeing until an RCU grace period has elapsed.
+ */
+static inline void list_del_bidir_rcu(struct list_head *entry)
+{
+	__list_del_entry(entry);
+}
+
+/**
  * hlist_del_init_rcu - deletes entry from hash list with re-initialization
  * @n: the element to delete from the hash list.
  *
@@ -286,6 +316,22 @@ static inline void list_splice_init_rcu(struct list_head *list,
 		&pos->member != (head); \
 		pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
 
+/**
+ * list_for_each_entry_reverse_rcu - iterate backwards over rcu list of given
+ * type
+ *
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_struct within the struct.
+ *
+ * This list-traversal primitive may safely run concurrently with
+ * the _rcu list-mutation primitives such as list_add_rcu()
+ * as long as the traversal is guarded by rcu_read_lock().
+ */
+#define list_for_each_entry_reverse_rcu(pos, head, member) \
+	for (pos = list_entry_rcu((head)->prev, typeof(*pos), member); \
+		&pos->member != (head); \
+		pos = list_entry_rcu(pos->member.prev, typeof(*pos), member))
 
 /**
  * list_for_each_continue_rcu

[RFC PATCH 2/6] notifiers: Convert notifier chain to circular doubly linked-list

[Srivatsa S. Bhat]

In order to support invoking the notifiers in the reverse order, we need to
be able to traverse the callback chain in both directions. So convert the
notifier list into a circular doubly linked list.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/mips/powertv/powertv_setup.c                |    2 
 arch/um/drivers/mconsole_kern.c                  |    2 
 arch/um/kernel/um_arch.c                         |    2 
 drivers/acpi/sleep.c                             |    2 
 drivers/char/ipmi/ipmi_msghandler.c              |    2 
 drivers/char/ipmi/ipmi_watchdog.c                |    4 -
 drivers/firmware/dcdbas.c                        |    2 
 drivers/md/md.c                                  |    2 
 drivers/net/ethernet/intel/igb/igb_main.c        |    2 
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    |    2 
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c |    2 
 drivers/staging/vt6655/device_main.c             |    2 
 include/linux/notifier.h                         |   23 +++--
 kernel/debug/debug_core.c                        |    2 
 kernel/notifier.c                                |   99 +++++++++++-----------
 kernel/trace/trace.c                             |    2 
 mm/page-writeback.c                              |    2 
 mm/vmstat.c                                      |    4 -
 18 files changed, 81 insertions(+), 77 deletions(-)

diff --git a/arch/mips/powertv/powertv_setup.c b/arch/mips/powertv/powertv_setup.c
index 3933c37..bade798 100644
--- a/arch/mips/powertv/powertv_setup.c
+++ b/arch/mips/powertv/powertv_setup.c
@@ -87,7 +87,7 @@ static void register_panic_notifier()
 {
 	static struct notifier_block panic_notifier = {
 		.notifier_call = panic_handler,
-		.next = NULL,
+		.list = LIST_HEAD_INIT(panic_notifier.list),
 		.priority	= INT_MAX
 	};
 	atomic_notifier_chain_register(&panic_notifier_list, &panic_notifier);
diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c
index 88e466b..dea89210 100644
--- a/arch/um/drivers/mconsole_kern.c
+++ b/arch/um/drivers/mconsole_kern.c
@@ -898,7 +898,7 @@ static int notify_panic(struct notifier_block *self, unsigned long unused1,
 
 static struct notifier_block panic_exit_notifier = {
 	.notifier_call 		= notify_panic,
-	.next 			= NULL,
+	.list			= LIST_HEAD_INIT(panic_exit_notifier.list),
 	.priority 		= 1
 };
 
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 4db8770..fdd2b78 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -243,7 +243,7 @@ static int panic_exit(struct notifier_block *self, unsigned long unused1,
 
 static struct notifier_block panic_exit_notifier = {
 	.notifier_call 		= panic_exit,
-	.next 			= NULL,
+	.list			= LIST_HEAD_INIT(panic_exit_notifier.list),
 	.priority 		= 0
 };
 
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 8856102..2fa6147 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -85,7 +85,7 @@ static int tts_notify_reboot(struct notifier_block *this,
 
 static struct notifier_block tts_notifier = {
 	.notifier_call	= tts_notify_reboot,
-	.next		= NULL,
+	.list		= LIST_HEAD_INIT(tts_notifier.list),
 	.priority	= 0,
 };
 
diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 2c29942..5a49739 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -4473,7 +4473,7 @@ static int panic_event(struct notifier_block *this,
 
 static struct notifier_block panic_block = {
 	.notifier_call	= panic_event,
-	.next		= NULL,
+	.list		= LIST_HEAD_INIT(panic_block.list),
 	.priority	= 200	/* priority: INT_MAX >= x >= 0 */
 };
 
diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index 7ed356e..e340cc2 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -1183,7 +1183,7 @@ static int wdog_reboot_handler(struct notifier_block *this,
 
 static struct notifier_block wdog_reboot_notifier = {
 	.notifier_call	= wdog_reboot_handler,
-	.next		= NULL,
+	.list		= LIST_HEAD_INIT(wdog_reboot_notifier.list),
 	.priority	= 0
 };
 
@@ -1212,7 +1212,7 @@ static int wdog_panic_handler(struct notifier_block *this,
 
 static struct notifier_block wdog_panic_notifier = {
 	.notifier_call	= wdog_panic_handler,
-	.next		= NULL,
+	.list		= LIST_HEAD_INIT(wdog_panic_notifier.list),
 	.priority	= 150	/* priority: INT_MAX >= x >= 0 */
 };
 
diff --git a/drivers/firmware/dcdbas.c b/drivers/firmware/dcdbas.c
index ea5ac2d..fcd2334 100644
--- a/drivers/firmware/dcdbas.c
+++ b/drivers/firmware/dcdbas.c
@@ -505,7 +505,7 @@ static int dcdbas_reboot_notify(struct notifier_block *nb, unsigned long code,
 
 static struct notifier_block dcdbas_reboot_nb = {
 	.notifier_call = dcdbas_reboot_notify,
-	.next = NULL,
+	.list = LIST_HEAD_INIT(dcdbas_reboot_nb.list),
 	.priority = INT_MIN
 };
 
diff --git a/drivers/md/md.c b/drivers/md/md.c
index a4c219e..166c917 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8437,7 +8437,7 @@ static int md_notify_reboot(struct notifier_block *this,
 
 static struct notifier_block md_notifier = {
 	.notifier_call	= md_notify_reboot,
-	.next		= NULL,
+	.list		= LIST_HEAD_INIT(md_notifier.list),
 	.priority	= INT_MAX, /* before any real devices */
 };
 
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index dd3bfe8..8bd87bb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -197,7 +197,7 @@ static void igb_shutdown(struct pci_dev *);
 static int igb_notify_dca(struct notifier_block *, unsigned long, void *);
 static struct notifier_block dca_notifier = {
 	.notifier_call	= igb_notify_dca,
-	.next		= NULL,
+	.list		= LIST_HEAD_INIT(dca_notifier.list),
 	.priority	= 0
 };
 #endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 18ca3bc..2f460ff 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -124,7 +124,7 @@ static int ixgbe_notify_dca(struct notifier_block *, unsigned long event,
 			    void *p);
 static struct notifier_block dca_notifier = {
 	.notifier_call = ixgbe_notify_dca,
-	.next          = NULL,
+	.list          = LIST_HEAD_INIT(dca_notifier.list),
 	.priority      = 0
 };
 #endif
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 90153fc..9514d81 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -4197,7 +4197,7 @@ myri10ge_notify_dca(struct notifier_block *nb, unsigned long event, void *p)
 
 static struct notifier_block myri10ge_dca_notifier = {
 	.notifier_call = myri10ge_notify_dca,
-	.next = NULL,
+	.list = LIST_HEAD_INIT(myri10ge_dca_notifier.list),
 	.priority = 0,
 };
 #endif				/* CONFIG_MYRI10GE_DCA */
diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 3e8283c..c71c2a5 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -313,7 +313,7 @@ static int viawget_suspend(struct pci_dev *pcid, pm_message_t state);
 static int viawget_resume(struct pci_dev *pcid);
 struct notifier_block device_notifier = {
 	.notifier_call = device_notify_reboot,
-	.next = NULL,
+	.list = LIST_HEAD_INIT(device_notifier.list),
 	.priority = 0,
 };
 #endif
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index d65746e..67f9a3a 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -49,40 +49,40 @@
 
 struct notifier_block {
 	int (*notifier_call)(struct notifier_block *, unsigned long, void *);
-	struct notifier_block __rcu *next;
+	struct list_head list;
 	int priority;
 };
 
 struct atomic_notifier_head {
 	spinlock_t lock;
-	struct notifier_block __rcu *head;
+	struct list_head list;
 };
 
 struct blocking_notifier_head {
 	struct rw_semaphore rwsem;
-	struct notifier_block __rcu *head;
+	struct list_head list;
 };
 
 struct raw_notifier_head {
-	struct notifier_block __rcu *head;
+	struct list_head list;
 };
 
 struct srcu_notifier_head {
 	struct mutex mutex;
 	struct srcu_struct srcu;
-	struct notifier_block __rcu *head;
+	struct list_head list;
 };
 
 #define ATOMIC_INIT_NOTIFIER_HEAD(name) do {	\
 		spin_lock_init(&(name)->lock);	\
-		(name)->head = NULL;		\
+		INIT_LIST_HEAD(&(name)->list);	\
 	} while (0)
 #define BLOCKING_INIT_NOTIFIER_HEAD(name) do {	\
 		init_rwsem(&(name)->rwsem);	\
-		(name)->head = NULL;		\
+		INIT_LIST_HEAD(&(name)->list);	\
 	} while (0)
 #define RAW_INIT_NOTIFIER_HEAD(name) do {	\
-		(name)->head = NULL;		\
+		 INIT_LIST_HEAD(&(name)->list);	\
 	} while (0)
 
 /* srcu_notifier_heads must be initialized and cleaned up dynamically */
@@ -92,12 +92,13 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
 
 #define ATOMIC_NOTIFIER_INIT(name) {				\
 		.lock = __SPIN_LOCK_UNLOCKED(name.lock),	\
-		.head = NULL }
+		.list = LIST_HEAD_INIT(name.list) }
 #define BLOCKING_NOTIFIER_INIT(name) {				\
 		.rwsem = __RWSEM_INITIALIZER((name).rwsem),	\
-		.head = NULL }
+		.list = LIST_HEAD_INIT(name.list) }
 #define RAW_NOTIFIER_INIT(name)	{				\
-		.head = NULL }
+		.list = LIST_HEAD_INIT(name.list) }
+
 /* srcu_notifier_heads cannot be initialized statically */
 
 #define ATOMIC_NOTIFIER_HEAD(name)				\
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 0557f24..9ac45ba 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -805,7 +805,7 @@ done:
 
 static struct notifier_block dbg_reboot_notifier = {
 	.notifier_call		= dbg_notify_reboot,
-	.next			= NULL,
+	.list			= LIST_HEAD_INIT(dbg_reboot_notifier.list),
 	.priority		= INT_MAX,
 };
 
diff --git a/kernel/notifier.c b/kernel/notifier.c
index 2d5cc4c..ad6feab 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -18,44 +18,48 @@ BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
  *	are layered on top of these, with appropriate locking added.
  */
 
-static int notifier_chain_register(struct notifier_block **nl,
+static int notifier_chain_register(struct list_head *nl,
 		struct notifier_block *n)
 {
-	while ((*nl) != NULL) {
-		if (n->priority > (*nl)->priority)
+	struct notifier_block *cur;
+
+	list_for_each_entry(cur, nl, list) {
+		if (n->priority > cur->priority)
 			break;
-		nl = &((*nl)->next);
 	}
-	n->next = *nl;
-	rcu_assign_pointer(*nl, n);
+
+	list_add_tail_rcu(&n->list, &cur->list);
 	return 0;
 }
 
-static int notifier_chain_cond_register(struct notifier_block **nl,
+static int notifier_chain_cond_register(struct list_head *nl,
 		struct notifier_block *n)
 {
-	while ((*nl) != NULL) {
-		if ((*nl) == n)
+	struct notifier_block *cur;
+
+	list_for_each_entry(cur, nl, list) {
+		if (cur == n)
 			return 0;
-		if (n->priority > (*nl)->priority)
+		if (n->priority > cur->priority)
 			break;
-		nl = &((*nl)->next);
 	}
-	n->next = *nl;
-	rcu_assign_pointer(*nl, n);
+
+	list_add_tail_rcu(&n->list, &cur->list);
 	return 0;
 }
 
-static int notifier_chain_unregister(struct notifier_block **nl,
+static int notifier_chain_unregister(struct list_head *nl,
 		struct notifier_block *n)
 {
-	while ((*nl) != NULL) {
-		if ((*nl) == n) {
-			rcu_assign_pointer(*nl, n->next);
+	struct notifier_block *cur;
+
+	list_for_each_entry(cur, nl, list) {
+		if (cur == n) {
+			list_del_bidir_rcu(&cur->list);
 			return 0;
 		}
-		nl = &((*nl)->next);
 	}
+
 	return -ENOENT;
 }
 
@@ -71,22 +75,20 @@ static int notifier_chain_unregister(struct notifier_block **nl,
  *	@returns:	notifier_call_chain returns the value returned by the
  *			last notifier function called.
  */
-static int __kprobes notifier_call_chain(struct notifier_block **nl,
+static int __kprobes notifier_call_chain(struct list_head *nl,
 					unsigned long val, void *v,
 					int nr_to_call,	int *nr_calls)
 {
 	int ret = NOTIFY_DONE;
-	struct notifier_block *nb, *next_nb;
-
-	nb = rcu_dereference_raw(*nl);
+	struct notifier_block *nb;
 
-	while (nb && nr_to_call) {
-		next_nb = rcu_dereference_raw(nb->next);
+	list_for_each_entry_rcu(nb, nl, list) {
+		if (!nr_to_call)
+			break;
 
 #ifdef CONFIG_DEBUG_NOTIFIERS
 		if (unlikely(!func_ptr_is_kernel_text(nb->notifier_call))) {
 			WARN(1, "Invalid notifier called!");
-			nb = next_nb;
 			continue;
 		}
 #endif
@@ -97,9 +99,9 @@ static int __kprobes notifier_call_chain(struct notifier_block **nl,
 
 		if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
 			break;
-		nb = next_nb;
 		nr_to_call--;
 	}
+
 	return ret;
 }
 
@@ -124,7 +126,7 @@ int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
 	int ret;
 
 	spin_lock_irqsave(&nh->lock, flags);
-	ret = notifier_chain_register(&nh->head, n);
+	ret = notifier_chain_register(&nh->list, n);
 	spin_unlock_irqrestore(&nh->lock, flags);
 	return ret;
 }
@@ -146,7 +148,7 @@ int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
 	int ret;
 
 	spin_lock_irqsave(&nh->lock, flags);
-	ret = notifier_chain_unregister(&nh->head, n);
+	ret = notifier_chain_unregister(&nh->list, n);
 	spin_unlock_irqrestore(&nh->lock, flags);
 	synchronize_rcu();
 	return ret;
@@ -179,7 +181,7 @@ int __kprobes __atomic_notifier_call_chain(struct atomic_notifier_head *nh,
 	int ret;
 
 	rcu_read_lock();
-	ret = notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
+	ret = notifier_call_chain(&nh->list, val, v, nr_to_call, nr_calls);
 	rcu_read_unlock();
 	return ret;
 }
@@ -218,10 +220,10 @@ int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
 	 * such times we must not call down_write().
 	 */
 	if (unlikely(system_state == SYSTEM_BOOTING))
-		return notifier_chain_register(&nh->head, n);
+		return notifier_chain_register(&nh->list, n);
 
 	down_write(&nh->rwsem);
-	ret = notifier_chain_register(&nh->head, n);
+	ret = notifier_chain_register(&nh->list, n);
 	up_write(&nh->rwsem);
 	return ret;
 }
@@ -244,7 +246,7 @@ int blocking_notifier_chain_cond_register(struct blocking_notifier_head *nh,
 	int ret;
 
 	down_write(&nh->rwsem);
-	ret = notifier_chain_cond_register(&nh->head, n);
+	ret = notifier_chain_cond_register(&nh->list, n);
 	up_write(&nh->rwsem);
 	return ret;
 }
@@ -271,10 +273,10 @@ int blocking_notifier_chain_unregister(struct blocking_notifier_head *nh,
 	 * such times we must not call down_write().
 	 */
 	if (unlikely(system_state == SYSTEM_BOOTING))
-		return notifier_chain_unregister(&nh->head, n);
+		return notifier_chain_unregister(&nh->list, n);
 
 	down_write(&nh->rwsem);
-	ret = notifier_chain_unregister(&nh->head, n);
+	ret = notifier_chain_unregister(&nh->list, n);
 	up_write(&nh->rwsem);
 	return ret;
 }
@@ -305,13 +307,14 @@ int __blocking_notifier_call_chain(struct blocking_notifier_head *nh,
 	int ret = NOTIFY_DONE;
 
 	/*
-	 * We check the head outside the lock, but if this access is
-	 * racy then it does not matter what the result of the test
-	 * is, we re-check the list after having taken the lock anyway:
+	 * We check whether the list is empty outside the lock, but if
+	 * this access is racy then it does not matter what the result
+	 * of the test is, we re-check the list after having taken the
+	 * lock anyway.
 	 */
-	if (rcu_dereference_raw(nh->head)) {
+	if (!list_empty(&nh->list)) {
 		down_read(&nh->rwsem);
-		ret = notifier_call_chain(&nh->head, val, v, nr_to_call,
+		ret = notifier_call_chain(&nh->list, val, v, nr_to_call,
 					nr_calls);
 		up_read(&nh->rwsem);
 	}
@@ -344,7 +347,7 @@ EXPORT_SYMBOL_GPL(blocking_notifier_call_chain);
 int raw_notifier_chain_register(struct raw_notifier_head *nh,
 		struct notifier_block *n)
 {
-	return notifier_chain_register(&nh->head, n);
+	return notifier_chain_register(&nh->list, n);
 }
 EXPORT_SYMBOL_GPL(raw_notifier_chain_register);
 
@@ -361,7 +364,7 @@ EXPORT_SYMBOL_GPL(raw_notifier_chain_register);
 int raw_notifier_chain_unregister(struct raw_notifier_head *nh,
 		struct notifier_block *n)
 {
-	return notifier_chain_unregister(&nh->head, n);
+	return notifier_chain_unregister(&nh->list, n);
 }
 EXPORT_SYMBOL_GPL(raw_notifier_chain_unregister);
 
@@ -388,7 +391,7 @@ int __raw_notifier_call_chain(struct raw_notifier_head *nh,
 			      unsigned long val, void *v,
 			      int nr_to_call, int *nr_calls)
 {
-	return notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
+	return notifier_call_chain(&nh->list, val, v, nr_to_call, nr_calls);
 }
 EXPORT_SYMBOL_GPL(__raw_notifier_call_chain);
 
@@ -425,10 +428,10 @@ int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
 	 * such times we must not call mutex_lock().
 	 */
 	if (unlikely(system_state == SYSTEM_BOOTING))
-		return notifier_chain_register(&nh->head, n);
+		return notifier_chain_register(&nh->list, n);
 
 	mutex_lock(&nh->mutex);
-	ret = notifier_chain_register(&nh->head, n);
+	ret = notifier_chain_register(&nh->list, n);
 	mutex_unlock(&nh->mutex);
 	return ret;
 }
@@ -455,10 +458,10 @@ int srcu_notifier_chain_unregister(struct srcu_notifier_head *nh,
 	 * such times we must not call mutex_lock().
 	 */
 	if (unlikely(system_state == SYSTEM_BOOTING))
-		return notifier_chain_unregister(&nh->head, n);
+		return notifier_chain_unregister(&nh->list, n);
 
 	mutex_lock(&nh->mutex);
-	ret = notifier_chain_unregister(&nh->head, n);
+	ret = notifier_chain_unregister(&nh->list, n);
 	mutex_unlock(&nh->mutex);
 	synchronize_srcu(&nh->srcu);
 	return ret;
@@ -491,7 +494,7 @@ int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
 	int idx;
 
 	idx = srcu_read_lock(&nh->srcu);
-	ret = notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
+	ret = notifier_call_chain(&nh->list, val, v, nr_to_call, nr_calls);
 	srcu_read_unlock(&nh->srcu, idx);
 	return ret;
 }
@@ -521,7 +524,7 @@ void srcu_init_notifier_head(struct srcu_notifier_head *nh)
 	mutex_init(&nh->mutex);
 	if (init_srcu_struct(&nh->srcu) < 0)
 		BUG();
-	nh->head = NULL;
+	INIT_LIST_HEAD(&nh->list);
 }
 EXPORT_SYMBOL_GPL(srcu_init_notifier_head);
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index a7fa070..9feadb8 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4876,7 +4876,7 @@ static int trace_panic_handler(struct notifier_block *this,
 
 static struct notifier_block trace_panic_notifier = {
 	.notifier_call  = trace_panic_handler,
-	.next           = NULL,
+	.list           = LIST_HEAD_INIT(trace_panic_notifier.list),
 	.priority       = 150   /* priority: INT_MAX >= x >= 0 */
 };
 
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 93d8d2f..4135c16 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1583,7 +1583,7 @@ ratelimit_handler(struct notifier_block *self, unsigned long u, void *v)
 
 static struct notifier_block __cpuinitdata ratelimit_nb = {
 	.notifier_call	= ratelimit_handler,
-	.next		= NULL,
+	.list		= LIST_HEAD_INIT(ratelimit_nb.list),
 };
 
 /*
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 1bbbbd9..ad67df0 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1196,8 +1196,8 @@ static int __cpuinit vmstat_cpuup_callback(struct notifier_block *nfb,
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __cpuinitdata vmstat_notifier =
-	{ &vmstat_cpuup_callback, NULL, 0 };
+static struct notifier_block __cpuinitdata vmstat_notifier = {
+	&vmstat_cpuup_callback, LIST_HEAD_INIT(vmstat_notifier.list), 0 };
 #endif
 
 static int __init setup_vmstat(void)

[RFC PATCH 3/6] notifiers: Add support for reverse invocation of notifier chains

[Srivatsa S. Bhat]

In certain scenarios, it is useful to be able to invoke notifiers in the
reverse order. One such example is CPU hotplug, where we would like to
invoke the notifiers in one order during CPU online and in the reverse
order during CPU offline. So add support for reverse invocation of
notifier chains.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/notifier.h |    4 ++
 kernel/notifier.c        |   82 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 67f9a3a..4626d17 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -145,8 +145,12 @@ extern int __blocking_notifier_call_chain(struct blocking_notifier_head *nh,
 	unsigned long val, void *v, int nr_to_call, int *nr_calls);
 extern int raw_notifier_call_chain(struct raw_notifier_head *nh,
 		unsigned long val, void *v);
+extern int raw_notifier_call_chain_reverse(struct raw_notifier_head *nh,
+		unsigned long val, void *v);
 extern int __raw_notifier_call_chain(struct raw_notifier_head *nh,
 	unsigned long val, void *v, int nr_to_call, int *nr_calls);
+extern int __raw_notifier_call_chain_reverse(struct raw_notifier_head *nh,
+	unsigned long val, void *v, int nr_to_call, int *nr_calls);
 extern int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
 		unsigned long val, void *v);
 extern int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
diff --git a/kernel/notifier.c b/kernel/notifier.c
index ad6feab..536f32c 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -105,6 +105,50 @@ static int __kprobes notifier_call_chain(struct list_head *nl,
 	return ret;
 }
 
+/**
+ * notifier_call_chain_reverse - Informs the registered notifiers about an
+ * event, by invoking the notifiers in the reverse order.
+ *
+ *	@nl:		Pointer to head of the blocking notifier chain
+ *	@val:		Value passed unmodified to notifier function
+ *	@v:		Pointer passed unmodified to notifier function
+ *	@nr_to_call:	Number of notifier functions to be called. Don't care
+ *			value of this parameter is -1.
+ *	@nr_calls:	Records the number of notifications sent. Don't care
+ *			value of this field is NULL.
+ *	@returns:	notifier_call_chain_reverse returns the value returned
+ *			by the last notifier function called.
+ */
+static int __kprobes notifier_call_chain_reverse(struct list_head *nl,
+					unsigned long val, void *v,
+					int nr_to_call,	int *nr_calls)
+{
+	int ret = NOTIFY_DONE;
+	struct notifier_block *nb;
+
+	list_for_each_entry_reverse_rcu(nb, nl, list) {
+		if (!nr_to_call)
+			break;
+
+#ifdef CONFIG_DEBUG_NOTIFIERS
+		if (unlikely(!func_ptr_is_kernel_text(nb->notifier_call))) {
+			WARN(1, "Invalid notifier called!");
+			continue;
+		}
+#endif
+		ret = nb->notifier_call(nb, val, v);
+
+		if (nr_calls)
+			(*nr_calls)++;
+
+		if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
+			break;
+		nr_to_call--;
+	}
+
+	return ret;
+}
+
 /*
  *	Atomic notifier chain routines.  Registration and unregistration
  *	use a spinlock, and call_chain is synchronized by RCU (no locks).
@@ -402,6 +446,44 @@ int raw_notifier_call_chain(struct raw_notifier_head *nh,
 }
 EXPORT_SYMBOL_GPL(raw_notifier_call_chain);
 
+
+/**
+ *	__raw_notifier_call_chain_reverse - Call functions in a raw notifier
+ *	chain in the reverse order
+ *
+ *	@nh: Pointer to head of the raw notifier chain
+ *	@val: Value passed unmodified to notifier function
+ *	@v: Pointer passed unmodified to notifier function
+ *	@nr_to_call: See comment for notifier_call_chain_reverse.
+ *	@nr_calls: See comment for notifier_call_chain_reverse
+ *
+ *	Calls each function in a notifier chain in turn, in the reverse order.
+ *	The functions run in an undefined context.
+ *	All locking must be provided by the caller.
+ *
+ *	If the return value of the notifier can be and'ed
+ *	with %NOTIFY_STOP_MASK then __raw_notifier_call_chain_reverse()
+ *	will return immediately, with the return value of
+ *	the notifier function which halted execution.
+ *	Otherwise the return value is the return value
+ *	of the last notifier function called.
+ */
+int __raw_notifier_call_chain_reverse(struct raw_notifier_head *nh,
+			      unsigned long val, void *v,
+			      int nr_to_call, int *nr_calls)
+{
+	return notifier_call_chain_reverse(&nh->list, val, v,
+						nr_to_call, nr_calls);
+}
+EXPORT_SYMBOL_GPL(__raw_notifier_call_chain_reverse);
+
+int raw_notifier_call_chain_reverse(struct raw_notifier_head *nh,
+		unsigned long val, void *v)
+{
+	return __raw_notifier_call_chain_reverse(nh, val, v, -1, NULL);
+}
+EXPORT_SYMBOL_GPL(raw_notifier_call_chain_reverse);
+
 /*
  *	SRCU notifier chain routines.    Registration and unregistration
  *	use a mutex, and call_chain is synchronized by SRCU (no locks).

[RFC PATCH 4/6] sched, cpuset: Prepare scheduler and cpuset CPU hotplug callbacks for reverse invocation

[Srivatsa S. Bhat]

Some of the CPU hotplug callbacks of the scheduler and cpuset infrastructure are
intertwined in an interesting way. The scheduler's sched_cpu_[in]active()
callbacks and cpuset's cpuset_cpu_[in]active() callbacks have the following
documented dependency:

The sched_cpu_active() callback must be the first callback to run, and
should be immediately followed by cpuset_cpu_active() to update the
cpusets and the sched domains. This ordering (sched followed by cpuset)
needs to be honored in both the CPU online *and* the CPU offline paths.
Hence its not straightforward to convert these callbacks to the reverse
invocation model, because, a plain conversion would result in the problem
explained below.

In general, if 2 notifiers A and B expect to be -always- called in the order
A followed by B, ie., during both CPU online and CPU offline, then we can't
ensure that easily because, when we do reverse invocation, we get the
following call path:

Event        |     Invocation order
-------------|---------------------
CPU online:  | A (high priority), B (low priority)
CPU offline: | B (low priority), A (high priority)

So this breaks the requirement for A and B. We see this ordering requirement
in the case of the scheduler and cpusets.

So, to solve this, club the 2 callbacks together as a unit, so that they
are always invoked as a unit, which means, forward or reverse, the requirement
is satisfied. In this case, since the 2 callbacks are quite related, it
doesn't break semantics/readability if we club them together, which is a good
thing!

There is a one more aspect that we need to take care of while clubbing the two
callbacks. During boot, the scheduler is initialized in two phases:
sched_init(), which happens before SMP initialization (and hence *before* the
non-boot CPUs are booted up), and sched_init_smp(), which happens after SMP
initialization (and hence *after* the non-boot CPUs are booted).

In the original code, the cpuset callbacks are registered during
sched_init_smp(), which means that while starting the non-boot CPUs, only the
scheduler callbacks are invoked, not the cpuset ones. So in order to keep
this intact even after clubbing the 2 callbacks, we need to be able to find out
if we are running post-SMP init or if we are running pre-SMP early boot code,
to decide whether to pass on control to the cpuset callback or not. So introduce
a flag 'sched_smp_init_complete' that gets set after the scheduler is
initialized for SMP. This would help us in making that decision.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/cpu.h |   16 ++++-----
 kernel/sched/core.c |   89 ++++++++++++++++++++++++++++++---------------------
 2 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index ce7a074..255b889 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -55,20 +55,18 @@ extern ssize_t arch_print_cpu_modalias(struct device *dev,
  */
 enum {
 	/*
-	 * SCHED_ACTIVE marks a cpu which is coming up active during
-	 * CPU_ONLINE and CPU_DOWN_FAILED and must be the first
-	 * notifier.  CPUSET_ACTIVE adjusts cpuset according to
-	 * cpu_active mask right after SCHED_ACTIVE.  During
-	 * CPU_DOWN_PREPARE, SCHED_INACTIVE and CPUSET_INACTIVE are
-	 * ordered in the similar way.
+	 * SCHED_ACTIVE marks a cpu which is coming up active during CPU_ONLINE
+	 * and CPU_DOWN_FAILED and must be the first notifier.  It then passes
+	 * control to the cpuset_cpu_active() notifier which adjusts cpusets
+	 * according to cpu_active mask. During CPU_DOWN_PREPARE, SCHED_INACTIVE
+	 * marks the cpu as inactive and passes control to the
+	 * cpuset_cpu_inactive() notifier in a similar way.
 	 *
 	 * This ordering guarantees consistent cpu_active mask and
 	 * migration behavior to all cpu notifiers.
 	 */
 	CPU_PRI_SCHED_ACTIVE	= INT_MAX,
-	CPU_PRI_CPUSET_ACTIVE	= INT_MAX - 1,
-	CPU_PRI_SCHED_INACTIVE	= INT_MIN + 1,
-	CPU_PRI_CPUSET_INACTIVE	= INT_MIN,
+	CPU_PRI_SCHED_INACTIVE	= INT_MIN,
 
 	/* migration should happen before other stuff but after perf */
 	CPU_PRI_PERF		= 20,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d5594a4..9ccebdd 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -280,6 +280,7 @@ const_debug unsigned int sysctl_sched_time_avg = MSEC_PER_SEC;
 unsigned int sysctl_sched_rt_period = 1000000;
 
 __read_mostly int scheduler_running;
+static bool __read_mostly sched_smp_init_complete;
 
 /*
  * part of the period that we allow rt tasks to run in us.
@@ -5505,29 +5506,75 @@ static struct notifier_block __cpuinitdata migration_notifier = {
 	.priority = CPU_PRI_MIGRATION,
 };
 
+/*
+ * Update cpusets according to cpu_active mask.  If cpusets are
+ * disabled, cpuset_update_active_cpus() becomes a simple wrapper
+ * around partition_sched_domains().
+ */
+static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
+			     void *hcpu)
+{
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_ONLINE:
+	case CPU_DOWN_FAILED:
+		cpuset_update_active_cpus();
+		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
+static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
+			       void *hcpu)
+{
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_DOWN_PREPARE:
+		cpuset_update_active_cpus();
+		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
 static int __cpuinit sched_cpu_active(struct notifier_block *nfb,
 				      unsigned long action, void *hcpu)
 {
+	int ret;
+
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_STARTING:
 	case CPU_DOWN_FAILED:
 		set_cpu_active((long)hcpu, true);
-		return NOTIFY_OK;
+		ret = NOTIFY_OK;
+		break;
 	default:
-		return NOTIFY_DONE;
+		ret = NOTIFY_DONE;
 	}
+
+	if (likely(sched_smp_init_complete))
+		return cpuset_cpu_active(nfb, action, hcpu);
+
+	return ret;
 }
 
 static int __cpuinit sched_cpu_inactive(struct notifier_block *nfb,
 					unsigned long action, void *hcpu)
 {
+	int ret;
+
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_DOWN_PREPARE:
 		set_cpu_active((long)hcpu, false);
-		return NOTIFY_OK;
+		ret = NOTIFY_OK;
+		break;
 	default:
-		return NOTIFY_DONE;
+		ret = NOTIFY_DONE;
 	}
+
+	if (likely(sched_smp_init_complete))
+		return cpuset_cpu_inactive(nfb, action, hcpu);
+
+	return ret;
 }
 
 static int __init migration_init(void)
@@ -6967,36 +7014,6 @@ match2:
 	mutex_unlock(&sched_domains_mutex);
 }
 
-/*
- * Update cpusets according to cpu_active mask.  If cpusets are
- * disabled, cpuset_update_active_cpus() becomes a simple wrapper
- * around partition_sched_domains().
- */
-static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
-			     void *hcpu)
-{
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_ONLINE:
-	case CPU_DOWN_FAILED:
-		cpuset_update_active_cpus();
-		return NOTIFY_OK;
-	default:
-		return NOTIFY_DONE;
-	}
-}
-
-static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
-			       void *hcpu)
-{
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_DOWN_PREPARE:
-		cpuset_update_active_cpus();
-		return NOTIFY_OK;
-	default:
-		return NOTIFY_DONE;
-	}
-}
-
 void __init sched_init_smp(void)
 {
 	cpumask_var_t non_isolated_cpus;
@@ -7015,9 +7032,6 @@ void __init sched_init_smp(void)
 	mutex_unlock(&sched_domains_mutex);
 	put_online_cpus();
 
-	hotcpu_notifier(cpuset_cpu_active, CPU_PRI_CPUSET_ACTIVE);
-	hotcpu_notifier(cpuset_cpu_inactive, CPU_PRI_CPUSET_INACTIVE);
-
 	/* RT runtime code needs to handle some hotplug events */
 	hotcpu_notifier(update_runtime, 0);
 
@@ -7030,6 +7044,7 @@ void __init sched_init_smp(void)
 	free_cpumask_var(non_isolated_cpus);
 
 	init_sched_rt_class();
+	sched_smp_init_complete = true;
 }
 #else
 void __init sched_init_smp(void)

[RFC PATCH 5/6] sched, perf: Prepare migration and perf CPU hotplug callbacks for reverse invocation

[Srivatsa S. Bhat]

While dealing with reverse invocation of callbacks during CPU offline, we
get an opportunity to revisit some of the reasons behind the existing callback
invocation orders and how they would fit into the new reverse invocation model
(which poses its own constraints and challenges).

It is documented that the perf and migration CPU hotplug callbacks must be
run pretty early (ie., before the normal callbacks that have priority 0 and
lower)... and also that the perf callbacks must be run before the migration
one. Again, at first glance, it looks like the "Notifier A must be followed
by B, always, in both cpu online and cpu offline paths" rule. However, looking
a bit closely at the code, it appears that this requirement is true mainly for
the CPU online path, and not for the CPU offline path.

In the CPU offline path, some of the perf callbacks deal with low-level
registers, whereas the migration callback deals with the scheduler runqueues
and stuff, which look quite unrelated. Also, there are quite a few priority 0
callbacks that deal with low-level arch-specific-cpu-disable stuff in the
CPU down path.

All in all, it appears that the requirement can be restated as follows:

CPU online path:
Run the perf callbacks early, followed by the migration callback and later
run the priority 0 and other callbacks as usual.

CPU offline path:
Run the migration callback early, followed by the priority 0 callbacks and
later run the perf callbacks.

Keeping this in mind, adjust the perf and migration callbacks in preparation
for moving over to the reverse invocation model. That is, split up the
migration callback into CPU online and CPU offline components and assign
suitable priorities to them. This split would help us move over to the
reverse invocation model easily, since we would now have the necessary control
over both the paths (online and offline) to handle the ordering requirements
correctly.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/cpu.h |    3 ++-
 kernel/sched/core.c |   55 ++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 255b889..88de47d 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -70,7 +70,8 @@ enum {
 
 	/* migration should happen before other stuff but after perf */
 	CPU_PRI_PERF		= 20,
-	CPU_PRI_MIGRATION	= 10,
+	CPU_PRI_MIGRATION_UP	= 10,
+	CPU_PRI_MIGRATION_DOWN	= 9,
 	/* bring up workqueues before normal notifiers and down after */
 	CPU_PRI_WORKQUEUE_UP	= 5,
 	CPU_PRI_WORKQUEUE_DOWN	= -5,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9ccebdd..6a511f2 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5444,12 +5444,10 @@ static void set_rq_offline(struct rq *rq)
 	}
 }
 
-/*
- * migration_call - callback that gets triggered when a CPU is added.
- * Here we can start up the necessary migration thread for the new CPU.
- */
+/* cpu_up_migration_call - callback that gets triggered when a CPU is added. */
 static int __cpuinit
-migration_call(struct notifier_block *nfb, unsigned long action, void *hcpu)
+cpu_up_migration_call(struct notifier_block *nfb, unsigned long action,
+								void *hcpu)
 {
 	int cpu = (long)hcpu;
 	unsigned long flags;
@@ -5471,6 +5469,36 @@ migration_call(struct notifier_block *nfb, unsigned long action, void *hcpu)
 		}
 		raw_spin_unlock_irqrestore(&rq->lock, flags);
 		break;
+	}
+
+	update_max_interval();
+
+	return NOTIFY_OK;
+}
+
+/*
+ * Register at high priority so that task migration (migrate_tasks)
+ * happens before everything else.  This has to be lower priority than
+ * the notifier in the perf_event subsystem, though.
+ */
+static struct notifier_block __cpuinitdata cpu_up_migration_notifier = {
+	.notifier_call = cpu_up_migration_call,
+	.priority = CPU_PRI_MIGRATION_UP,
+};
+
+/*
+ * cpu_down_migration_call - callback that gets triggered when a CPU is
+ * removed.
+ */
+static int __cpuinit
+cpu_down_migration_call(struct notifier_block *nfb, unsigned long action,
+								void *hcpu)
+{
+	int cpu = (long)hcpu;
+	unsigned long flags;
+	struct rq *rq = cpu_rq(cpu);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
 
 #ifdef CONFIG_HOTPLUG_CPU
 	case CPU_DYING:
@@ -5497,13 +5525,13 @@ migration_call(struct notifier_block *nfb, unsigned long action, void *hcpu)
 }
 
 /*
- * Register at high priority so that task migration (migrate_all_tasks)
+ * Register at high priority so that task migration (migrate_tasks)
  * happens before everything else.  This has to be lower priority than
  * the notifier in the perf_event subsystem, though.
  */
-static struct notifier_block __cpuinitdata migration_notifier = {
-	.notifier_call = migration_call,
-	.priority = CPU_PRI_MIGRATION,
+static struct notifier_block __cpuinitdata cpu_down_migration_notifier = {
+	.notifier_call = cpu_down_migration_call,
+	.priority = CPU_PRI_MIGRATION_DOWN,
 };
 
 /*
@@ -5583,10 +5611,13 @@ static int __init migration_init(void)
 	int err;
 
 	/* Initialize migration for the boot CPU */
-	err = migration_call(&migration_notifier, CPU_UP_PREPARE, cpu);
+	err = cpu_up_migration_call(&cpu_up_migration_notifier,
+						CPU_UP_PREPARE, cpu);
 	BUG_ON(err == NOTIFY_BAD);
-	migration_call(&migration_notifier, CPU_ONLINE, cpu);
-	register_cpu_notifier(&migration_notifier);
+	cpu_up_migration_call(&cpu_up_migration_notifier, CPU_ONLINE, cpu);
+	register_cpu_notifier(&cpu_up_migration_notifier);
+
+	register_cpu_notifier(&cpu_down_migration_notifier);
 
 	/* Register cpu active notifiers */
 	cpu_notifier(sched_cpu_active, CPU_PRI_SCHED_ACTIVE);

[RFC PATCH 6/6] CPU hotplug: Invoke CPU offline notifiers in reverse order

[Srivatsa S. Bhat]

During CPU hotplug, we want to create the following effect:
 * During CPU online, the CPU incrementally grows the number of services
   it offers.
 * During CPU offline, the services are incrementally retired, in the
   reverse order of their growth during CPU online.

To achieve the above, invoke the registered CPU hotplug notifiers in the
reverse order during CPU offline, with respect to the order in which they
were invoked during CPU online.

This approach helps in making the hotplug path a lot more logically
coherent: Services are started in a well-known order, perhaps with dependencies
between them, while bringing up a CPU. While offlining the CPU, we honor
many of the dependencies automatically by just backtracking the way we came;
that is, by invoking the notifiers in the reverse order. Thus, this model of
reverse notifier invocation helps us in avoiding the need for excessively
messing with the notifier priorities while trying to get the ordering right.

Notes:
1. The workqueue code becomes simpler, since it now needs just a single CPU
hotplug callback.
2. The scheduler's sched_cpu_[in]active() callbacks can also be merged into
a single callback.
3. On similar lines, the cpusets' cpuset_cpu_[in]active() callbacks can also
be merged.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/cpu.h |   37 ++++++++++++++++++++++++-------------
 kernel/cpu.c        |   34 ++++++++++++++++++++++++++--------
 kernel/sched/core.c |   48 ++++++++++++++----------------------------------
 kernel/workqueue.c  |   30 +++++++++++-------------------
 4 files changed, 75 insertions(+), 74 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 88de47d..b27a046 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -55,26 +55,37 @@ extern ssize_t arch_print_cpu_modalias(struct device *dev,
  */
 enum {
 	/*
-	 * SCHED_ACTIVE marks a cpu which is coming up active during CPU_ONLINE
-	 * and CPU_DOWN_FAILED and must be the first notifier.  It then passes
-	 * control to the cpuset_cpu_active() notifier which adjusts cpusets
-	 * according to cpu_active mask. During CPU_DOWN_PREPARE, SCHED_INACTIVE
-	 * marks the cpu as inactive and passes control to the
-	 * cpuset_cpu_inactive() notifier in a similar way.
+	 * sched_cpu_notify() marks a cpu which is coming up active during
+	 * CPU_ONLINE and CPU_DOWN_FAILED and must be the first notifier.  It
+	 * then passes control to the cpuset_cpu_active() notifier which adjusts
+	 * cpusets according to cpu_active mask. During CPU_DOWN_PREPARE,
+	 * sched_cpu_notify() marks the cpu as inactive and passes control to
+	 * the cpuset_cpu_inactive() notifier in a similar way.
 	 *
 	 * This ordering guarantees consistent cpu_active mask and
 	 * migration behavior to all cpu notifiers.
 	 */
-	CPU_PRI_SCHED_ACTIVE	= INT_MAX,
-	CPU_PRI_SCHED_INACTIVE	= INT_MIN,
+	CPU_PRI_SCHED		= INT_MAX,
 
-	/* migration should happen before other stuff but after perf */
+	/*
+	 * Migration should happen before other stuff but after perf,
+	 * in the CPU online path.
+	 */
 	CPU_PRI_PERF		= 20,
 	CPU_PRI_MIGRATION_UP	= 10,
-	CPU_PRI_MIGRATION_DOWN	= 9,
-	/* bring up workqueues before normal notifiers and down after */
-	CPU_PRI_WORKQUEUE_UP	= 5,
-	CPU_PRI_WORKQUEUE_DOWN	= -5,
+
+	/*
+	 * Bring up workqueues before normal notifiers in the CPU online
+	 * path. Do the opposite in the CPU offline path.
+	 */
+	CPU_PRI_WORKQUEUE	= 5,
+
+	/*
+	 * In the CPU offline path, the notifiers are invoked in the reverse
+	 * order. So use a numerically low value for the priority, to ensure
+	 * that we get invoked early.
+	 */
+	CPU_PRI_MIGRATION_DOWN	= -10,
 };
 
 #define CPU_ONLINE		0x0002 /* CPU (unsigned)v is up */
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 8ab33ae..30444c2 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -155,16 +155,32 @@ static int __cpu_notify(unsigned long val, void *v, int nr_to_call,
 	return notifier_to_errno(ret);
 }
 
+static int __cpu_notify_reverse(unsigned long val, void *v, int nr_to_call,
+			int *nr_calls)
+{
+	int ret;
+
+	ret = __raw_notifier_call_chain_reverse(&cpu_chain, val, v,
+							nr_to_call, nr_calls);
+
+	return notifier_to_errno(ret);
+}
+
 static int cpu_notify(unsigned long val, void *v)
 {
 	return __cpu_notify(val, v, -1, NULL);
 }
 
+static int cpu_notify_reverse(unsigned long val, void *v)
+{
+	return __cpu_notify_reverse(val, v, -1, NULL);
+}
+
 #ifdef CONFIG_HOTPLUG_CPU
 
-static void cpu_notify_nofail(unsigned long val, void *v)
+static void cpu_notify_reverse_nofail(unsigned long val, void *v)
 {
-	BUG_ON(cpu_notify(val, v));
+	BUG_ON(cpu_notify_reverse(val, v));
 }
 EXPORT_SYMBOL(register_cpu_notifier);
 
@@ -249,7 +265,7 @@ static int __ref take_cpu_down(void *_param)
 	if (err < 0)
 		return err;
 
-	cpu_notify(CPU_DYING | param->mod, param->hcpu);
+	cpu_notify_reverse(CPU_DYING | param->mod, param->hcpu);
 	return 0;
 }
 
@@ -272,10 +288,12 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen)
 
 	cpu_hotplug_begin();
 
-	err = __cpu_notify(CPU_DOWN_PREPARE | mod, hcpu, -1, &nr_calls);
+	err = __cpu_notify_reverse(CPU_DOWN_PREPARE | mod, hcpu, -1,
+								&nr_calls);
 	if (err) {
 		nr_calls--;
-		__cpu_notify(CPU_DOWN_FAILED | mod, hcpu, nr_calls, NULL);
+		__cpu_notify_reverse(CPU_DOWN_FAILED | mod, hcpu, nr_calls,
+									NULL);
 		printk("%s: attempt to take down CPU %u failed\n",
 				__func__, cpu);
 		goto out_release;
@@ -286,7 +304,7 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen)
 	if (err) {
 		/* CPU didn't die: tell everyone.  Can't complain. */
 		smpboot_unpark_threads(cpu);
-		cpu_notify_nofail(CPU_DOWN_FAILED | mod, hcpu);
+		cpu_notify_reverse_nofail(CPU_DOWN_FAILED | mod, hcpu);
 		goto out_release;
 	}
 	BUG_ON(cpu_online(cpu));
@@ -305,14 +323,14 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen)
 	__cpu_die(cpu);
 
 	/* CPU is completely dead: tell everyone.  Too late to complain. */
-	cpu_notify_nofail(CPU_DEAD | mod, hcpu);
+	cpu_notify_reverse_nofail(CPU_DEAD | mod, hcpu);
 
 	check_for_tasks(cpu);
 
 out_release:
 	cpu_hotplug_done();
 	if (!err)
-		cpu_notify_nofail(CPU_POST_DEAD | mod, hcpu);
+		cpu_notify_reverse_nofail(CPU_POST_DEAD | mod, hcpu);
 	return err;
 }
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 6a511f2..9dbcb11 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5477,6 +5477,9 @@ cpu_up_migration_call(struct notifier_block *nfb, unsigned long action,
 }
 
 /*
+ * CPU online path: Notifiers are invoked in normal order; that is, notifiers
+ * with numerically higher priorities are invoked first.
+ *
  * Register at high priority so that task migration (migrate_tasks)
  * happens before everything else.  This has to be lower priority than
  * the notifier in the perf_event subsystem, though.
@@ -5525,9 +5528,11 @@ cpu_down_migration_call(struct notifier_block *nfb, unsigned long action,
 }
 
 /*
- * Register at high priority so that task migration (migrate_tasks)
- * happens before everything else.  This has to be lower priority than
- * the notifier in the perf_event subsystem, though.
+ * CPU offline path: Notifiers are invoked in reverse order; that is,
+ * notifiers with numerically lower priorities are invoked first.
+ *
+ * Register at low priority so that task migration (migrate_tasks)
+ * happens before everything else.
  */
 static struct notifier_block __cpuinitdata cpu_down_migration_notifier = {
 	.notifier_call = cpu_down_migration_call,
@@ -5539,32 +5544,22 @@ static struct notifier_block __cpuinitdata cpu_down_migration_notifier = {
  * disabled, cpuset_update_active_cpus() becomes a simple wrapper
  * around partition_sched_domains().
  */
-static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
+static int cpuset_cpu_notify(struct notifier_block *nfb, unsigned long action,
 			     void *hcpu)
 {
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_ONLINE:
 	case CPU_DOWN_FAILED:
-		cpuset_update_active_cpus();
-		return NOTIFY_OK;
-	default:
-		return NOTIFY_DONE;
-	}
-}
-
-static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
-			       void *hcpu)
-{
-	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_DOWN_PREPARE:
 		cpuset_update_active_cpus();
 		return NOTIFY_OK;
+
 	default:
 		return NOTIFY_DONE;
 	}
 }
 
-static int __cpuinit sched_cpu_active(struct notifier_block *nfb,
+static int __cpuinit sched_cpu_notify(struct notifier_block *nfb,
 				      unsigned long action, void *hcpu)
 {
 	int ret;
@@ -5575,32 +5570,18 @@ static int __cpuinit sched_cpu_active(struct notifier_block *nfb,
 		set_cpu_active((long)hcpu, true);
 		ret = NOTIFY_OK;
 		break;
-	default:
-		ret = NOTIFY_DONE;
-	}
-
-	if (likely(sched_smp_init_complete))
-		return cpuset_cpu_active(nfb, action, hcpu);
-
-	return ret;
-}
 
-static int __cpuinit sched_cpu_inactive(struct notifier_block *nfb,
-					unsigned long action, void *hcpu)
-{
-	int ret;
-
-	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_DOWN_PREPARE:
 		set_cpu_active((long)hcpu, false);
 		ret = NOTIFY_OK;
 		break;
+
 	default:
 		ret = NOTIFY_DONE;
 	}
 
 	if (likely(sched_smp_init_complete))
-		return cpuset_cpu_inactive(nfb, action, hcpu);
+		return cpuset_cpu_notify(nfb, action, hcpu);
 
 	return ret;
 }
@@ -5620,8 +5601,7 @@ static int __init migration_init(void)
 	register_cpu_notifier(&cpu_down_migration_notifier);
 
 	/* Register cpu active notifiers */
-	cpu_notifier(sched_cpu_active, CPU_PRI_SCHED_ACTIVE);
-	cpu_notifier(sched_cpu_inactive, CPU_PRI_SCHED_INACTIVE);
+	cpu_notifier(sched_cpu_notify, CPU_PRI_SCHED);
 
 	return 0;
 }
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 471996a..5fba1da 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3429,16 +3429,24 @@ static void gcwq_unbind_fn(struct work_struct *work)
 }
 
 /*
+ * CPU online:
  * Workqueues should be brought up before normal priority CPU notifiers.
- * This will be registered high priority CPU notifier.
+ *
+ * CPU offline:
+ * Workqueues should be brought down after normal priority CPU notifiers.
+ *
+ * This will be registered as high priority CPU notifier. Then, the automatic
+ * reverse invocation of notifiers in the CPU offline path will satisfy both
+ * the above requirements.
  */
-static int __devinit workqueue_cpu_up_callback(struct notifier_block *nfb,
+static int __devinit workqueue_cpu_callback(struct notifier_block *nfb,
 					       unsigned long action,
 					       void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
 	struct global_cwq *gcwq = get_gcwq(cpu);
 	struct worker_pool *pool;
+	struct work_struct unbind_work;
 
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_UP_PREPARE:
@@ -3465,22 +3473,7 @@ static int __devinit workqueue_cpu_up_callback(struct notifier_block *nfb,
 		rebind_workers(gcwq);
 		gcwq_release_management_and_unlock(gcwq);
 		break;
-	}
-	return NOTIFY_OK;
-}
 
-/*
- * Workqueues should be brought down after normal priority CPU notifiers.
- * This will be registered as low priority CPU notifier.
- */
-static int __devinit workqueue_cpu_down_callback(struct notifier_block *nfb,
-						 unsigned long action,
-						 void *hcpu)
-{
-	unsigned int cpu = (unsigned long)hcpu;
-	struct work_struct unbind_work;
-
-	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_DOWN_PREPARE:
 		/* unbinding should happen on the local CPU */
 		INIT_WORK_ONSTACK(&unbind_work, gcwq_unbind_fn);
@@ -3686,8 +3679,7 @@ static int __init init_workqueues(void)
 	unsigned int cpu;
 	int i;
 
-	cpu_notifier(workqueue_cpu_up_callback, CPU_PRI_WORKQUEUE_UP);
-	cpu_notifier(workqueue_cpu_down_callback, CPU_PRI_WORKQUEUE_DOWN);
+	cpu_notifier(workqueue_cpu_callback, CPU_PRI_WORKQUEUE);
 
 	/* initialize gcwqs */
 	for_each_gcwq_cpu(cpu) {

Re: [RFC PATCH 6/6] CPU hotplug: Invoke CPU offline notifiers in reverse order

[Tejun Heo]

Hello,

On Wed, Jul 25, 2012 at 05:25:13PM +0530, Srivatsa S. Bhat wrote:
> During CPU hotplug, we want to create the following effect:
>  * During CPU online, the CPU incrementally grows the number of services
>    it offers.
>  * During CPU offline, the services are incrementally retired, in the
>    reverse order of their growth during CPU online.
> 
> To achieve the above, invoke the registered CPU hotplug notifiers in the
> reverse order during CPU offline, with respect to the order in which they
> were invoked during CPU online.
> 
> This approach helps in making the hotplug path a lot more logically
> coherent: Services are started in a well-known order, perhaps with dependencies
> between them, while bringing up a CPU. While offlining the CPU, we honor
> many of the dependencies automatically by just backtracking the way we came;
> that is, by invoking the notifiers in the reverse order. Thus, this model of
> reverse notifier invocation helps us in avoiding the need for excessively
> messing with the notifier priorities while trying to get the ordering right.
> 
> Notes:
> 1. The workqueue code becomes simpler, since it now needs just a single CPU
> hotplug callback.
> 2. The scheduler's sched_cpu_[in]active() callbacks can also be merged into
> a single callback.
> 3. On similar lines, the cpusets' cpuset_cpu_[in]active() callbacks can also
> be merged.
> 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

For workqueue part,

 Acked-by: Tejun Heo <tj@kernel.org>

Now that we can reverse-walk notifier chain, it probably is a good
idea to make it a bit smarter and call CANCELs in reverse order of
PREPAREs too.

Thanks.

-- 
tejun

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Alan Stern]

On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:

> Hi,
> 
> This patchset implements the approach of invoking the CPU hotplug callbacks
> (notifiers) in one order during CPU online and in the reverse order during CPU
> offline. The rationale behind this is that services for a CPU are started in a
> particular order (perhaps, with implicit dependencies between them) while
> bringing up the CPU, and hence, it makes sense to tear down the services in
> the opposite order, thereby honoring most of the dependencies automatically
> (and also correctly). This is explained in more detail in Patch 6.

This strongly suggests that a notifier chain may be the wrong mechanism
to use here.  Notifiers provide only limited guarantees about ordering,
and it's hard to say much about the services a particular chain will
provide since callbacks can be added from anywhere.

Instead of adding all this complication to the notifier mechanism, how 
about using something else for CPU hotplug?

Alan Stern

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

On 07/25/2012 08:27 PM, Alan Stern wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> 
>> Hi,
>>
>> This patchset implements the approach of invoking the CPU hotplug callbacks
>> (notifiers) in one order during CPU online and in the reverse order during CPU
>> offline. The rationale behind this is that services for a CPU are started in a
>> particular order (perhaps, with implicit dependencies between them) while
>> bringing up the CPU, and hence, it makes sense to tear down the services in
>> the opposite order, thereby honoring most of the dependencies automatically
>> (and also correctly). This is explained in more detail in Patch 6.
> 
> This strongly suggests that a notifier chain may be the wrong mechanism
> to use here.  Notifiers provide only limited guarantees about ordering,
> and it's hard to say much about the services a particular chain will
> provide since callbacks can be added from anywhere.
> 

True, the ability to register any random callback from anywhere is still a
problem that we are fighting... The zillions of callbacks that we have today
makes the hotplug process quite entangled.. we can't even roll-back from a
failure easily!

> Instead of adding all this complication to the notifier mechanism, how 
> about using something else for CPU hotplug?
> 

The problem is that today, many different subsystems need to know about CPUs coming
up or going down.. And CPU hotplug is not atomic, it happens in stages, and the
coordination between those subsystems is what actually drives CPU hotplug, in a way.
At present, I think that the best we can do is to redesign the hotplug code such that
the number of callbacks that are needed can be reduced to a minimum amount and then
have good control over what those callbacks do. For example, Thomas Gleixner posted
the park/unpark patchset[1], which not only speeds-up CPU hotplug by avoiding destruction
and creation of per-cpu kthreads on every hotplug operation, but also gets rid of quite
a few notifiers by providing a framework to manage those per-cpu kthreads...

One of the other ideas to improve the hotplug notifier stuff that came up during some
of the discussions was to implement explicit dependency tracking between the notifiers
and perhaps get rid of the priority numbers that are currently being used to provide
some sort of ordering between the callbacks. Links to some of the related discussions
are provided below.

References:
[1]. http://thread.gmane.org/gmane.linux.kernel/1328391/focus=1328391
[2]. https://lkml.org/lkml/2012/4/5/379
[3]. http://thread.gmane.org/gmane.linux.kernel.cross-arch/13656/focus=13678
[4]. http://thread.gmane.org/gmane.linux.kernel.cross-arch/13656/focus=14208
[5]. http://thread.gmane.org/gmane.linux.kernel.cross-arch/13656/focus=14214 

Regards,
Srivatsa S. Bhat

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Alan Stern]

On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:

> On 07/25/2012 08:27 PM, Alan Stern wrote:
> > On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> > 
> >> Hi,
> >>
> >> This patchset implements the approach of invoking the CPU hotplug callbacks
> >> (notifiers) in one order during CPU online and in the reverse order during CPU
> >> offline. The rationale behind this is that services for a CPU are started in a
> >> particular order (perhaps, with implicit dependencies between them) while
> >> bringing up the CPU, and hence, it makes sense to tear down the services in
> >> the opposite order, thereby honoring most of the dependencies automatically
> >> (and also correctly). This is explained in more detail in Patch 6.
> > 
> > This strongly suggests that a notifier chain may be the wrong mechanism
> > to use here.  Notifiers provide only limited guarantees about ordering,
> > and it's hard to say much about the services a particular chain will
> > provide since callbacks can be added from anywhere.
> > 
> 
> True, the ability to register any random callback from anywhere is still a
> problem that we are fighting... The zillions of callbacks that we have today
> makes the hotplug process quite entangled.. we can't even roll-back from a
> failure easily!
> 
> > Instead of adding all this complication to the notifier mechanism, how 
> > about using something else for CPU hotplug?
> > 
> 
> The problem is that today, many different subsystems need to know about CPUs coming
> up or going down.. And CPU hotplug is not atomic, it happens in stages, and the
> coordination between those subsystems is what actually drives CPU hotplug, in a way.

All this reinforces the idea that notifiers are the wrong mechanism for 
CPU hotplug.

> At present, I think that the best we can do is to redesign the hotplug code such that
> the number of callbacks that are needed can be reduced to a minimum amount and then
> have good control over what those callbacks do. For example, Thomas Gleixner posted
> the park/unpark patchset[1], which not only speeds-up CPU hotplug by avoiding destruction
> and creation of per-cpu kthreads on every hotplug operation, but also gets rid of quite
> a few notifiers by providing a framework to manage those per-cpu kthreads...

I think the best you can do is stop using notifiers and use something 
else instead.  For example, a simple set of function calls (assuming 
you know beforehand what callbacks need to be invoked).

> One of the other ideas to improve the hotplug notifier stuff that came up during some
> of the discussions was to implement explicit dependency tracking between the notifiers
> and perhaps get rid of the priority numbers that are currently being used to provide
> some sort of ordering between the callbacks. Links to some of the related discussions
> are provided below.

This seems like misplaced over-engineering.

Alan Stern

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

On 07/25/2012 09:40 PM, Alan Stern wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> 
>> On 07/25/2012 08:27 PM, Alan Stern wrote:
>>> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
>>>
>>>> Hi,
>>>>
>>>> This patchset implements the approach of invoking the CPU hotplug callbacks
>>>> (notifiers) in one order during CPU online and in the reverse order during CPU
>>>> offline. The rationale behind this is that services for a CPU are started in a
>>>> particular order (perhaps, with implicit dependencies between them) while
>>>> bringing up the CPU, and hence, it makes sense to tear down the services in
>>>> the opposite order, thereby honoring most of the dependencies automatically
>>>> (and also correctly). This is explained in more detail in Patch 6.
>>>
>>> This strongly suggests that a notifier chain may be the wrong mechanism
>>> to use here.  Notifiers provide only limited guarantees about ordering,
>>> and it's hard to say much about the services a particular chain will
>>> provide since callbacks can be added from anywhere.
>>>
>>
>> True, the ability to register any random callback from anywhere is still a
>> problem that we are fighting... The zillions of callbacks that we have today
>> makes the hotplug process quite entangled.. we can't even roll-back from a
>> failure easily!
>>
>>> Instead of adding all this complication to the notifier mechanism, how 
>>> about using something else for CPU hotplug?
>>>
>>
>> The problem is that today, many different subsystems need to know about CPUs coming
>> up or going down.. And CPU hotplug is not atomic, it happens in stages, and the
>> coordination between those subsystems is what actually drives CPU hotplug, in a way.
> 
> All this reinforces the idea that notifiers are the wrong mechanism for 
> CPU hotplug.
> 
>> At present, I think that the best we can do is to redesign the hotplug code such that
>> the number of callbacks that are needed can be reduced to a minimum amount and then
>> have good control over what those callbacks do. For example, Thomas Gleixner posted
>> the park/unpark patchset[1], which not only speeds-up CPU hotplug by avoiding destruction
>> and creation of per-cpu kthreads on every hotplug operation, but also gets rid of quite
>> a few notifiers by providing a framework to manage those per-cpu kthreads...
> 
> I think the best you can do is stop using notifiers and use something 
> else instead.  For example, a simple set of function calls (assuming 
> you know beforehand what callbacks need to be invoked).

Hmmm.. So, that way we would know exactly what we are doing, and we would have complete
control over the ordering and rollback (if needed).. Sounds like a good idea! But I'm not
sure off hand whether there are any serious hurdles to that! (other than the fact that
perhaps we'll end up having way too many function calls inside the core code)...
I'll try and see how that can be implemented. Thanks!

> 
>> One of the other ideas to improve the hotplug notifier stuff that came up during some
>> of the discussions was to implement explicit dependency tracking between the notifiers
>> and perhaps get rid of the priority numbers that are currently being used to provide
>> some sort of ordering between the callbacks. Links to some of the related discussions
>> are provided below.
> 
> This seems like misplaced over-engineering.
>

Heh ;-)

Regards,
Srivatsa S. Bhat

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Paul E. McKenney]

On Wed, Jul 25, 2012 at 12:10:41PM -0400, Alan Stern wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> 
> > On 07/25/2012 08:27 PM, Alan Stern wrote:
> > > On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> > > 
> > >> Hi,
> > >>
> > >> This patchset implements the approach of invoking the CPU hotplug callbacks
> > >> (notifiers) in one order during CPU online and in the reverse order during CPU
> > >> offline. The rationale behind this is that services for a CPU are started in a
> > >> particular order (perhaps, with implicit dependencies between them) while
> > >> bringing up the CPU, and hence, it makes sense to tear down the services in
> > >> the opposite order, thereby honoring most of the dependencies automatically
> > >> (and also correctly). This is explained in more detail in Patch 6.
> > > 
> > > This strongly suggests that a notifier chain may be the wrong mechanism
> > > to use here.  Notifiers provide only limited guarantees about ordering,
> > > and it's hard to say much about the services a particular chain will
> > > provide since callbacks can be added from anywhere.
> > > 
> > 
> > True, the ability to register any random callback from anywhere is still a
> > problem that we are fighting... The zillions of callbacks that we have today
> > makes the hotplug process quite entangled.. we can't even roll-back from a
> > failure easily!
> > 
> > > Instead of adding all this complication to the notifier mechanism, how 
> > > about using something else for CPU hotplug?
> > > 
> > 
> > The problem is that today, many different subsystems need to know about CPUs coming
> > up or going down.. And CPU hotplug is not atomic, it happens in stages, and the
> > coordination between those subsystems is what actually drives CPU hotplug, in a way.
> 
> All this reinforces the idea that notifiers are the wrong mechanism for 
> CPU hotplug.
> 
> > At present, I think that the best we can do is to redesign the hotplug code such that
> > the number of callbacks that are needed can be reduced to a minimum amount and then
> > have good control over what those callbacks do. For example, Thomas Gleixner posted
> > the park/unpark patchset[1], which not only speeds-up CPU hotplug by avoiding destruction
> > and creation of per-cpu kthreads on every hotplug operation, but also gets rid of quite
> > a few notifiers by providing a framework to manage those per-cpu kthreads...
> 
> I think the best you can do is stop using notifiers and use something 
> else instead.  For example, a simple set of function calls (assuming 
> you know beforehand what callbacks need to be invoked).

Unfortunately, we don't know beforehand.  There are loadable kernel
modules that need to be informed of CPU hotplug events.  So we do need
some runtime way to register to be notified CPU-hotplug events.

If it weren't for the modules, we might be able to use another data
section and get a contiguous list of functions that need to be called.
I suppose we could have such sections for modules and link/unlink them
at module load/unload time, but it is not at all clear to me that this
would be simpler than using notifiers.

But always happy to learn -- any specific suggestions?

> > One of the other ideas to improve the hotplug notifier stuff that came up during some
> > of the discussions was to implement explicit dependency tracking between the notifiers
> > and perhaps get rid of the priority numbers that are currently being used to provide
> > some sort of ordering between the callbacks. Links to some of the related discussions
> > are provided below.
> 
> This seems like misplaced over-engineering.

In fact, we have deferred dependency tracking.  If I remember correctly,
the main motivation was to execute notifiers in parallel, but the fraction
of time that hotplug spends executing notifiers proved to be quite small.
At this point, I would be surprised if we ever do much with dependency
tracking.

Again, any specific suggestions for improvement?

							Thanx, Paul

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Alan Stern]

On Wed, 25 Jul 2012, Paul E. McKenney wrote:

> > I think the best you can do is stop using notifiers and use something 
> > else instead.  For example, a simple set of function calls (assuming 
> > you know beforehand what callbacks need to be invoked).
> 
> Unfortunately, we don't know beforehand.  There are loadable kernel
> modules that need to be informed of CPU hotplug events.  So we do need
> some runtime way to register to be notified CPU-hotplug events.
> 
> If it weren't for the modules, we might be able to use another data
> section and get a contiguous list of functions that need to be called.
> I suppose we could have such sections for modules and link/unlink them
> at module load/unload time, but it is not at all clear to me that this
> would be simpler than using notifiers.
> 
> But always happy to learn -- any specific suggestions?

Not knowing the details of CPU hotplug, it's hard to answer in anything 
but generalities.  :-)  (And FYI, I have no intention of learning the 
details of CPU hotplug -- there's enough on my plate already.)

Still, one possibility would be to organize matters by adding a couple
of steps to Thomas's list that would be devoted purely to callbacks and
associating a separate notifier chain to each of these steps.  That way
most of the important work would be kept in the core kernel but modules
could still step in where needed.  The different notifier chains would 
then be strictly ordered with respect to each other, but events within 
a particular chain would not have any specified ordering properties.

Also, Thomas may have mentioned something about having separate 
callbacks for the CPU-up and CPU-down events, instead of relying on one 
list that would be called forwards and backwards.

> > > One of the other ideas to improve the hotplug notifier stuff that came up during some
> > > of the discussions was to implement explicit dependency tracking between the notifiers
> > > and perhaps get rid of the priority numbers that are currently being used to provide
> > > some sort of ordering between the callbacks. Links to some of the related discussions
> > > are provided below.
> > 
> > This seems like misplaced over-engineering.
> 
> In fact, we have deferred dependency tracking.  If I remember correctly,
> the main motivation was to execute notifiers in parallel, but the fraction
> of time that hotplug spends executing notifiers proved to be quite small.
> At this point, I would be surprised if we ever do much with dependency
> tracking.
> 
> Again, any specific suggestions for improvement?

If it works okay without dependency tracking, leave well enough alone.  
:-)

Alan Stern

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Thomas Gleixner]

On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> On 07/25/2012 08:27 PM, Alan Stern wrote:
> One of the other ideas to improve the hotplug notifier stuff that came up during some
> of the discussions was to implement explicit dependency tracking between the notifiers
> and perhaps get rid of the priority numbers that are currently being used to provide
> some sort of ordering between the callbacks. Links to some of the related discussions
> are provided below.

The current code which brings up/down a CPU (mostly architecture
specific) code is comnpletely asymetric.

We really want a fully symetric state machine here, which also gives
us the proper invocation points for the other subsystems callbacks.

While I thought about having a full dependency tracking system, I'm
quite convinced by now, that hotplug is a rather linear sequence which
does not provide much room for paralell setup/teardown.

At least we should start with a simple linear chain.

The problem with the current notifiers is, that we only have ordering
for a few specific callbacks, but we don't have the faintest idea in
which order all other random stuff is brought up and torn down.

So I started experimenting with the following:

struct hotplug_event {
       int (*bring_up)(unsigned int cpu);
       int (*tear_down)(unsigned int cpu);
};

enum hotplug_events {
     CPU_HOTPLUG_START,
     CPU_HOTPLUG_CREATE_THREADS,
     CPU_HOTPLUG_INIT_TIMERS,
     ...
     CPU_HOTPLUG_KICK_CPU,
     ...
     CPU_HOTPLUG_START_THREADS,
     ...
     CPU_HOTPLUG_SET_ONLINE,
     ...
     CPU_HOTPLUG_MAX_EVENTS,
};

Now I have two arrays:

struct hotplug_event hotplug_events_bp[CPU_HOTPLUG_MAX_EVENTS];
struct hotplug_event hotplug_events_ap[CPU_HOTPLUG_MAX_EVENTS];
   
The _bp one is the list of events which are executed on the active cpu
and the _ap ones are those executed on the hotplugged cpu.

The core code advances the events in sync steps, so both BP and AP can
issue a stop on the process and cause a rollback.

Most of the callbacks can be added to the arrays at compile time, just
the stuff which is in modules requires an register/unregister
interface.

Though in any case the enum gives us a very explicit ordering of
setup/teardown, so rollback or partial online/offline should be simple
to achieve.

The only drawback is that it will prevent out of tree modules to use
the hotplug infrastructure, but I really couldn't care less.

Thoughts?

	tglx

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

On 07/25/2012 10:00 PM, Thomas Gleixner wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
>> On 07/25/2012 08:27 PM, Alan Stern wrote:
>> One of the other ideas to improve the hotplug notifier stuff that came up during some
>> of the discussions was to implement explicit dependency tracking between the notifiers
>> and perhaps get rid of the priority numbers that are currently being used to provide
>> some sort of ordering between the callbacks. Links to some of the related discussions
>> are provided below.
> 
> The current code which brings up/down a CPU (mostly architecture
> specific) code is comnpletely asymetric.
> 
> We really want a fully symetric state machine here, which also gives
> us the proper invocation points for the other subsystems callbacks.
> 

Right..

> While I thought about having a full dependency tracking system, I'm
> quite convinced by now, that hotplug is a rather linear sequence which
> does not provide much room for paralell setup/teardown.
>

Pretty much, when considering hotplug of a single CPU.

(But when considering booting, Arjan had proposed (while discussing about his asynchronous
booting patch) that it would be good to split up physical vs logical parts of the
booting/hotplug, so that the physical part can happen in parallel with other CPUs, while
the logical online can be done serially, much later. Anyway, this is slightly off-topic
here, since we are mainly talking about hotplug of a single cpu here. I just thought of
putting a word about that here, since we are discussing hotplug redesign anyways..)
 
> At least we should start with a simple linear chain.
> 
> The problem with the current notifiers is, that we only have ordering
> for a few specific callbacks, but we don't have the faintest idea in
> which order all other random stuff is brought up and torn down.
> 

Right, and moreover there are some strange dependencies/ordering between some of
them considering facts such as, CPU_DOWN_PREPARE runs before CPU_DEAD for example,
no matter what priority you give to your callback.. Some callbacks seem to miss
this observation, IIRC.

> So I started experimenting with the following:
> 
> struct hotplug_event {
>        int (*bring_up)(unsigned int cpu);
>        int (*tear_down)(unsigned int cpu);
> };
> 
> enum hotplug_events {
>      CPU_HOTPLUG_START,
>      CPU_HOTPLUG_CREATE_THREADS,
>      CPU_HOTPLUG_INIT_TIMERS,
>      ...
>      CPU_HOTPLUG_KICK_CPU,
>      ...
>      CPU_HOTPLUG_START_THREADS,
>      ...
>      CPU_HOTPLUG_SET_ONLINE,
>      ...
>      CPU_HOTPLUG_MAX_EVENTS,
> };
> 
> Now I have two arrays:
> 
> struct hotplug_event hotplug_events_bp[CPU_HOTPLUG_MAX_EVENTS];
> struct hotplug_event hotplug_events_ap[CPU_HOTPLUG_MAX_EVENTS];
>    
> The _bp one is the list of events which are executed on the active cpu
> and the _ap ones are those executed on the hotplugged cpu.
> 
> The core code advances the events in sync steps, so both BP and AP can
> issue a stop on the process and cause a rollback.
> 

Looks like a nice design!

> Most of the callbacks can be added to the arrays at compile time, just
> the stuff which is in modules requires an register/unregister
> interface.
> 
> Though in any case the enum gives us a very explicit ordering of
> setup/teardown, so rollback or partial online/offline should be simple
> to achieve.
> 
> The only drawback is that it will prevent out of tree modules to use
> the hotplug infrastructure, but I really couldn't care less.
> 

Heh ;-)

Regards,
Srivatsa S. Bhat

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Thomas Gleixner]

On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> On 07/25/2012 10:00 PM, Thomas Gleixner wrote:
> > While I thought about having a full dependency tracking system, I'm
> > quite convinced by now, that hotplug is a rather linear sequence which
> > does not provide much room for paralell setup/teardown.
> >
> 
> Pretty much, when considering hotplug of a single CPU.
> 
> (But when considering booting, Arjan had proposed (while discussing
> about his asynchronous booting patch) that it would be good to split
> up physical vs logical parts of the booting/hotplug, so that the
> physical part can happen in parallel with other CPUs, while the
> logical online can be done serially, much later. Anyway, this is
> slightly off-topic here, since we are mainly talking about hotplug
> of a single cpu here. I just thought of putting a word about that
> here, since we are discussing hotplug redesign anyways..)

Well, the nice thing about having a proper state machine is that you
can tell the code to advance the BP only to the "kick the other cpu"
step, which is before the first sync point, so you can leave the state
there and continue with "bring it fully online" later. 

So that feature comes basically for free. :)

Thanks,

	tglx

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

On 07/26/2012 04:32 PM, Thomas Gleixner wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
>> On 07/25/2012 10:00 PM, Thomas Gleixner wrote:
>>> While I thought about having a full dependency tracking system, I'm
>>> quite convinced by now, that hotplug is a rather linear sequence which
>>> does not provide much room for paralell setup/teardown.
>>>
>>
>> Pretty much, when considering hotplug of a single CPU.
>>
>> (But when considering booting, Arjan had proposed (while discussing
>> about his asynchronous booting patch) that it would be good to split
>> up physical vs logical parts of the booting/hotplug, so that the
>> physical part can happen in parallel with other CPUs, while the
>> logical online can be done serially, much later. Anyway, this is
>> slightly off-topic here, since we are mainly talking about hotplug
>> of a single cpu here. I just thought of putting a word about that
>> here, since we are discussing hotplug redesign anyways..)
> 
> Well, the nice thing about having a proper state machine is that you
> can tell the code to advance the BP only to the "kick the other cpu"
> step, which is before the first sync point, so you can leave the state
> there and continue with "bring it fully online" later. 
> 
> So that feature comes basically for free. :)
> 

Ah! Nice :-)

Regards,
Srivatsa S. Bhat

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

On 07/25/2012 10:00 PM, Thomas Gleixner wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
>> On 07/25/2012 08:27 PM, Alan Stern wrote:
>> One of the other ideas to improve the hotplug notifier stuff that came up during some
>> of the discussions was to implement explicit dependency tracking between the notifiers
>> and perhaps get rid of the priority numbers that are currently being used to provide
>> some sort of ordering between the callbacks. Links to some of the related discussions
>> are provided below.
> 
> The current code which brings up/down a CPU (mostly architecture
> specific) code is comnpletely asymetric.
> 
> We really want a fully symetric state machine here, which also gives
> us the proper invocation points for the other subsystems callbacks.
> 
> While I thought about having a full dependency tracking system, I'm
> quite convinced by now, that hotplug is a rather linear sequence which
> does not provide much room for paralell setup/teardown.
> 
> At least we should start with a simple linear chain.
> 
> The problem with the current notifiers is, that we only have ordering
> for a few specific callbacks, but we don't have the faintest idea in
> which order all other random stuff is brought up and torn down.
> 
> So I started experimenting with the following:
> 
> struct hotplug_event {
>        int (*bring_up)(unsigned int cpu);
>        int (*tear_down)(unsigned int cpu);
> };
> 
> enum hotplug_events {
>      CPU_HOTPLUG_START,
>      CPU_HOTPLUG_CREATE_THREADS,
>      CPU_HOTPLUG_INIT_TIMERS,
>      ...
>      CPU_HOTPLUG_KICK_CPU,
>      ...
>      CPU_HOTPLUG_START_THREADS,
>      ...
>      CPU_HOTPLUG_SET_ONLINE,
>      ...
>      CPU_HOTPLUG_MAX_EVENTS,
> };
> 
> Now I have two arrays:
> 
> struct hotplug_event hotplug_events_bp[CPU_HOTPLUG_MAX_EVENTS];
> struct hotplug_event hotplug_events_ap[CPU_HOTPLUG_MAX_EVENTS];
>    
> The _bp one is the list of events which are executed on the active cpu
> and the _ap ones are those executed on the hotplugged cpu.
> 
> The core code advances the events in sync steps, so both BP and AP can
> issue a stop on the process and cause a rollback.

What exactly does "sync steps" mean in this context? Also, for the CPU
offline event, the event could start off with both the BP and the AP being
the same CPU.. Does this design take care of that case?

Regards,
Srivatsa S. Bhat

> 
> Most of the callbacks can be added to the arrays at compile time, just
> the stuff which is in modules requires an register/unregister
> interface.
> 
> Though in any case the enum gives us a very explicit ordering of
> setup/teardown, so rollback or partial online/offline should be simple
> to achieve.
> 
> The only drawback is that it will prevent out of tree modules to use
> the hotplug infrastructure, but I really couldn't care less.
> 
> Thoughts?

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Thomas Gleixner]

On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
> On 07/25/2012 10:00 PM, Thomas Gleixner wrote:
> > struct hotplug_event hotplug_events_bp[CPU_HOTPLUG_MAX_EVENTS];
> > struct hotplug_event hotplug_events_ap[CPU_HOTPLUG_MAX_EVENTS];
> >    
> > The _bp one is the list of events which are executed on the active cpu
> > and the _ap ones are those executed on the hotplugged cpu.
> > 
> > The core code advances the events in sync steps, so both BP and AP can
> > issue a stop on the process and cause a rollback.
> 
> What exactly does "sync steps" mean in this context? Also, for the CPU

Sync step means, that both sides need to synchronize - not at every
step, but at well defined synchronization points. You can't advance
the AP to online state unless the BP has done the preparatory stuff
already.

> offline event, the event could start off with both the BP and the AP being
> the same CPU.. Does this design take care of that case?

Once the AP leaves the state where tasks can be freely scheduled on
it, the take down thread migrates automagically. And that's one of the
first things I'm trying to do so the first synchronization point is
after that.

Thanks,

	tglx

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

On 07/26/2012 04:25 PM, Thomas Gleixner wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
>> On 07/25/2012 10:00 PM, Thomas Gleixner wrote:
>>> struct hotplug_event hotplug_events_bp[CPU_HOTPLUG_MAX_EVENTS];
>>> struct hotplug_event hotplug_events_ap[CPU_HOTPLUG_MAX_EVENTS];
>>>    
>>> The _bp one is the list of events which are executed on the active cpu
>>> and the _ap ones are those executed on the hotplugged cpu.
>>>
>>> The core code advances the events in sync steps, so both BP and AP can
>>> issue a stop on the process and cause a rollback.
>>
>> What exactly does "sync steps" mean in this context? Also, for the CPU
> 
> Sync step means, that both sides need to synchronize - not at every
> step, but at well defined synchronization points. You can't advance
> the AP to online state unless the BP has done the preparatory stuff
> already.
> 
>> offline event, the event could start off with both the BP and the AP being
>> the same CPU.. Does this design take care of that case?
> 
> Once the AP leaves the state where tasks can be freely scheduled on
> it, the take down thread migrates automagically. And that's one of the
> first things I'm trying to do so the first synchronization point is
> after that.
> 

Oh.. Ok.. Thanks for the explanation!

Regards,
Srivatsa S. Bhat

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Srivatsa S. Bhat]

On 07/25/2012 10:00 PM, Thomas Gleixner wrote:
> On Wed, 25 Jul 2012, Srivatsa S. Bhat wrote:
>> On 07/25/2012 08:27 PM, Alan Stern wrote:
>> One of the other ideas to improve the hotplug notifier stuff that came up during some
>> of the discussions was to implement explicit dependency tracking between the notifiers
>> and perhaps get rid of the priority numbers that are currently being used to provide
>> some sort of ordering between the callbacks. Links to some of the related discussions
>> are provided below.
> 
> The current code which brings up/down a CPU (mostly architecture
> specific) code is comnpletely asymetric.
> 
> We really want a fully symetric state machine here, which also gives
> us the proper invocation points for the other subsystems callbacks.
> 
> While I thought about having a full dependency tracking system, I'm
> quite convinced by now, that hotplug is a rather linear sequence which
> does not provide much room for paralell setup/teardown.
> 
> At least we should start with a simple linear chain.
> 
> The problem with the current notifiers is, that we only have ordering
> for a few specific callbacks, but we don't have the faintest idea in
> which order all other random stuff is brought up and torn down.
> 
> So I started experimenting with the following:
> 
> struct hotplug_event {
>        int (*bring_up)(unsigned int cpu);
>        int (*tear_down)(unsigned int cpu);
> };
> 
> enum hotplug_events {
>      CPU_HOTPLUG_START,
>      CPU_HOTPLUG_CREATE_THREADS,
>      CPU_HOTPLUG_INIT_TIMERS,
>      ...
>      CPU_HOTPLUG_KICK_CPU,
>      ...
>      CPU_HOTPLUG_START_THREADS,
>      ...
>      CPU_HOTPLUG_SET_ONLINE,
>      ...
>      CPU_HOTPLUG_MAX_EVENTS,
> };
> 
> Now I have two arrays:
> 
> struct hotplug_event hotplug_events_bp[CPU_HOTPLUG_MAX_EVENTS];
> struct hotplug_event hotplug_events_ap[CPU_HOTPLUG_MAX_EVENTS];
>    
> The _bp one is the list of events which are executed on the active cpu
> and the _ap ones are those executed on the hotplugged cpu.
> 
> The core code advances the events in sync steps, so both BP and AP can
> issue a stop on the process and cause a rollback.
> 
> Most of the callbacks can be added to the arrays at compile time, just
> the stuff which is in modules requires an register/unregister
> interface.
> 
> Though in any case the enum gives us a very explicit ordering of
> setup/teardown, so rollback or partial online/offline should be simple
> to achieve.
>

Is there anything I can help you with, in implementing this new design?

Regards,
Srivatsa S. Bhat

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Rusty Russell]

On Wed, 25 Jul 2012 18:30:41 +0200 (CEST), Thomas Gleixner <tglx@linutronix.de> wrote:
> The problem with the current notifiers is, that we only have ordering
> for a few specific callbacks, but we don't have the faintest idea in
> which order all other random stuff is brought up and torn down.
> 
> So I started experimenting with the following:
> 
> struct hotplug_event {
>        int (*bring_up)(unsigned int cpu);
>        int (*tear_down)(unsigned int cpu);
> };
> 
> enum hotplug_events {
>      CPU_HOTPLUG_START,
>      CPU_HOTPLUG_CREATE_THREADS,
>      CPU_HOTPLUG_INIT_TIMERS,
>      ...
>      CPU_HOTPLUG_KICK_CPU,
>      ...
>      CPU_HOTPLUG_START_THREADS,
>      ...
>      CPU_HOTPLUG_SET_ONLINE,
>      ...
>      CPU_HOTPLUG_MAX_EVENTS,
> };

This looks awfully like hardcoded a list of calls, without the
readability :)

OK, I finally got off my ass and looked at the different users of cpu
hotplug.  Some are just doing crazy stuff, but most seem to fall into
two types:

1) Hardware-style cpu callbacks (CPU_UP_PREPARE & CPU_DEAD)
2) Live cpu callbacks (CPU_ONLINE & CPU_DOWN_PREPARE)

I think this is what Srivatsa was referring to with "physical" and
"logical" parts.  Maybe we should explicitly split them, with the idea
that we'd automatically call the other one if we hit an error.

struct cpu_hotplug_physical {
       int (*coming)(unsigned int cpu);
       void (*gone)(unsigned int cpu);
};

struct cpu_hotplug_logical {
       void (*arrived)(unsigned int cpu);
       int (*going)(unsigned int cpu);
};

Several of the live cpu callbacks seem racy to me, since we could be
running userspace tasks before CPU_ONLINE.  It'd be nice to fix this,
too.

Anyway, if we get a model which fits 90%, we can always open-code the
tricky ones.

Cheers,
Rusty.

Re: [RFC PATCH 0/6] CPU hotplug: Reverse invocation of notifiers during CPU hotplug

[Thomas Gleixner]

On Fri, 27 Jul 2012, Rusty Russell wrote:

> On Wed, 25 Jul 2012 18:30:41 +0200 (CEST), Thomas Gleixner <tglx@linutronix.de> wrote:
> > The problem with the current notifiers is, that we only have ordering
> > for a few specific callbacks, but we don't have the faintest idea in
> > which order all other random stuff is brought up and torn down.
> > 
> > So I started experimenting with the following:
> > 
> > struct hotplug_event {
> >        int (*bring_up)(unsigned int cpu);
> >        int (*tear_down)(unsigned int cpu);
> > };
> > 
> > enum hotplug_events {
> >      CPU_HOTPLUG_START,
> >      CPU_HOTPLUG_CREATE_THREADS,
> >      CPU_HOTPLUG_INIT_TIMERS,
> >      ...
> >      CPU_HOTPLUG_KICK_CPU,
> >      ...
> >      CPU_HOTPLUG_START_THREADS,
> >      ...
> >      CPU_HOTPLUG_SET_ONLINE,
> >      ...
> >      CPU_HOTPLUG_MAX_EVENTS,
> > };
> 
> This looks awfully like hardcoded a list of calls, without the
> readability :)

I'd love to make it a list of calls, though we have module users of
cpu hotplug which makes a list of calls a tad hard.
 
> OK, I finally got off my ass and looked at the different users of cpu
> hotplug.  Some are just doing crazy stuff, but most seem to fall into
> two types:
> 
> 1) Hardware-style cpu callbacks (CPU_UP_PREPARE & CPU_DEAD)
> 2) Live cpu callbacks (CPU_ONLINE & CPU_DOWN_PREPARE)
> 
> I think this is what Srivatsa was referring to with "physical" and
> "logical" parts.  Maybe we should explicitly split them, with the idea
> that we'd automatically call the other one if we hit an error.
> 
> struct cpu_hotplug_physical {
>        int (*coming)(unsigned int cpu);
>        void (*gone)(unsigned int cpu);
> };
> 
> struct cpu_hotplug_logical {
>        void (*arrived)(unsigned int cpu);
>        int (*going)(unsigned int cpu);
> };
> 
> Several of the live cpu callbacks seem racy to me, since we could be
> running userspace tasks before CPU_ONLINE.  It'd be nice to fix this,
> too.

Yes, I know. I wan't to change that as well. The trick here is that we
can schedule per cpu stuff on a not fully online cpu and only when all
the callbacks have been executed allow user space tasks to be
scheduled on that newly online cpu.
 
Thanks,

	tglx