[RFC 0/4] Cpu-Hotplug: Use per subsystem hot-cpu mutexes.

[RFC 0/4] Cpu-Hotplug: Use per subsystem hot-cpu mutexes.

[Gautham R Shenoy]

Hello everyone,

Since 2.6.18-something, the community has been bugged by the problem to
provide a clean and a stable mechanism to postpone a cpu-hotplug event
as lock_cpu_hotplug was badly broken.

This is another proposal towards solving that problem. This one is 
along the lines of the solution provided in kernel/workqueue.c

Instead of having a global mechanism like lock_cpu_hotplug, 
we allow the subsytems to define their own per-subsystem hot cpu
mutexes. These would be taken(released) where ever we are currently 
calling lock_cpu_hotplug(unlock_cpu_hotplug).

Also, in the per-subsystem hotcpu callback function,we take
this mutex before we handle any pre-cpu-hotplug events and release
it once we finish handling the post-cpu-hotplug events. A standard
means for doing this has been provided in [PATCH 2/4] and demonstrated
in [PATCH 3/4].

The ordering of these per-subsystem mutexes might still prove to be a
problem, but hopefully lockdep should help us get out of that muddle.

The patch set to be applied against linux-2.6.19-rc5 is as follows:

[PATCH 1/4] :	Extend notifier_call_chain with an option to specify the
		number of notifications to be sent and also count the
		number of notifications actually sent.
		
[PATCH 2/4] :	Define events CPU_LOCK_ACQUIRE and CPU_LOCK_RELEASE
		and send out notifications for these in _cpu_up and
		_cpu_down. This would help us standardise the acquire and
		release of the subsystem locks in the hotcpu 
		callback functions of these subsystems.
		
[PATCH 3/4] :	Eliminate lock_cpu_hotplug from kernel/sched.c.
		
[PATCH 4/4] :	In workqueue_cpu_callback function, acquire(release) the
		workqueue_mutex while handling 
		CPU_LOCK_ACQUIRE(CPU_LOCK_RELEASE).

If the per-subsystem-locking approach survives the test of time,
we can expect a slow phasing out of lock_cpu_hotplug, which has not
yet been eliminated in these patches :)

Awaiting your feedback.

Thanks,
gautham.

PS: These patches are intended for post 2.6.19, since most of the warnings
with respect to cpu_hotplug_locking (including the cpufreq ones) seem to 
have disappeared in 2.6.19-rc5.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

[PATCH 1/4] Extend notifier_call_chain to count nr_calls made.

[Gautham R Shenoy]

Provide notifier_call_chain with an option to call only a specified number of 
notifiers and also record the number of call to notifiers made.

The need for this enhancement was identified in the post entitled 
"Slab - Eliminate lock_cpu_hotplug from slab" 
(http://lkml.org/lkml/2006/10/28/92) by Ravikiran G Thirumalai and 
Andrew Morton.

This patch adds two additional parameters to notifier_call_chain API namely 
 - int nr_to_calls : Number of notifier_functions to be called. 
 		     The don't care value is -1.

 - unsigned int *nr_calls : Records the total number of notifier_funtions 
			    called by notifier_call_chain. The don't care
			    value is NULL.

Credit : Andrew Morton <akpm@osdl.org> 
Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>

--
 include/linux/notifier.h |    8 +++
 kernel/sys.c             |   97 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 89 insertions(+), 16 deletions(-)

Index: hotplug/kernel/sys.c
===================================================================
--- hotplug.orig/kernel/sys.c
+++ hotplug/kernel/sys.c
@@ -134,19 +134,41 @@ static int notifier_chain_unregister(str
 	return -ENOENT;
 }
 
+/*
+ * notifier_call_chain - Informs the registered notifiers about an event.
+ *
+ *	@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.
+ *
+ * 	RETURN VALUE:	notifier_call_chain returns the value returned by the
+ *			last notifier function called.
+ */
+
 static int __kprobes notifier_call_chain(struct notifier_block **nl,
-		unsigned long val, void *v)
+					unsigned long val, void *v,
+					int nr_to_call,	unsigned int *nr_calls)
 {
 	int ret = NOTIFY_DONE;
 	struct notifier_block *nb, *next_nb;
 
 	nb = rcu_dereference(*nl);
-	while (nb) {
+
+	while (nb && nr_to_call) {
 		next_nb = rcu_dereference(nb->next);
 		ret = nb->notifier_call(nb, val, v);
+
+		if (nr_calls)
+			*nr_calls ++;
+
 		if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
 			break;
 		nb = next_nb;
+		nr_to_call--;
 	}
 	return ret;
 }
@@ -205,10 +227,13 @@ int atomic_notifier_chain_unregister(str
 EXPORT_SYMBOL_GPL(atomic_notifier_chain_unregister);
 
 /**
- *	atomic_notifier_call_chain - Call functions in an atomic notifier chain
+ *	__atomic_notifier_call_chain - Call functions in an atomic notifier
+ *				       chain
  *	@nh: Pointer to head of the atomic notifier chain
  *	@val: Value passed unmodified to notifier function
  *	@v: Pointer passed unmodified to notifier function
+ *	@nr_to_call: See the comment for notifier_call_chain.
+ *	@nr_calls: See the comment for notifier_call_chain.
  *
  *	Calls each function in a notifier chain in turn.  The functions
  *	run in an atomic context, so they must not block.
@@ -222,19 +247,27 @@ EXPORT_SYMBOL_GPL(atomic_notifier_chain_
  *	of the last notifier function called.
  */
  
-int __kprobes atomic_notifier_call_chain(struct atomic_notifier_head *nh,
-		unsigned long val, void *v)
+int __kprobes __atomic_notifier_call_chain(struct atomic_notifier_head *nh,
+					unsigned long val, void *v,
+					int nr_to_call, unsigned int *nr_calls)
 {
 	int ret;
 
 	rcu_read_lock();
-	ret = notifier_call_chain(&nh->head, val, v);
+	ret = notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
 	rcu_read_unlock();
 	return ret;
 }
 
-EXPORT_SYMBOL_GPL(atomic_notifier_call_chain);
+EXPORT_SYMBOL_GPL(__atomic_notifier_call_chain);
 
+int __kprobes atomic_notifier_call_chain(struct atomic_notifier_head *nh,
+		unsigned long val, void *v)
+{
+	return __atomic_notifier_call_chain(nh, val, v, -1, NULL);
+}
+
+EXPORT_SYMBOL_GPL(atomic_notifier_call_chain);
 /*
  *	Blocking notifier chain routines.  All access to the chain is
  *	synchronized by an rwsem.
@@ -304,10 +337,13 @@ int blocking_notifier_chain_unregister(s
 EXPORT_SYMBOL_GPL(blocking_notifier_chain_unregister);
 
 /**
- *	blocking_notifier_call_chain - Call functions in a blocking notifier chain
+ *	__blocking_notifier_call_chain - Call functions in a blocking notifier
+ *					 chain
  *	@nh: 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: See comment for notifier_call_chain.
+ *	@nr_calls: See comment for notifier_call_chain.
  *
  *	Calls each function in a notifier chain in turn.  The functions
  *	run in a process context, so they are allowed to block.
@@ -320,17 +356,26 @@ EXPORT_SYMBOL_GPL(blocking_notifier_chai
  *	of the last notifier function called.
  */
  
-int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
-		unsigned long val, void *v)
+int __blocking_notifier_call_chain(struct blocking_notifier_head *nh,
+				   unsigned long val, void *v,
+				   int nr_to_call, unsigned int * nr_calls)
 {
 	int ret;
 
 	down_read(&nh->rwsem);
-	ret = notifier_call_chain(&nh->head, val, v);
+	ret = notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
 	up_read(&nh->rwsem);
 	return ret;
 }
 
+EXPORT_SYMBOL_GPL(__blocking_notifier_call_chain);
+
+int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
+		unsigned long val, void *v)
+{
+	return __blocking_notifier_call_chain(nh, val, v, -1, NULL);
+}
+
 EXPORT_SYMBOL_GPL(blocking_notifier_call_chain);
 
 /*
@@ -376,10 +421,12 @@ int raw_notifier_chain_unregister(struct
 EXPORT_SYMBOL_GPL(raw_notifier_chain_unregister);
 
 /**
- *	raw_notifier_call_chain - Call functions in a raw notifier chain
+ *	__raw_notifier_call_chain - Call functions in a raw notifier chain
  *	@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.
+ *	@nr_calls: See comment for notifier_call_chain
  *
  *	Calls each function in a notifier chain in turn.  The functions
  *	run in an undefined context.
@@ -393,10 +440,19 @@ EXPORT_SYMBOL_GPL(raw_notifier_chain_unr
  *	of the last notifier function called.
  */
 
+int __raw_notifier_call_chain(struct raw_notifier_head *nh,
+			      unsigned long val, void *v,
+			      int nr_to_call, unsigned int *nr_calls)
+{
+	return notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
+}
+
+EXPORT_SYMBOL_GPL(__raw_notifier_call_chain);
+
 int raw_notifier_call_chain(struct raw_notifier_head *nh,
 		unsigned long val, void *v)
 {
-	return notifier_call_chain(&nh->head, val, v);
+	return __raw_notifier_call_chain(nh, val, v, -1, NULL);
 }
 
 EXPORT_SYMBOL_GPL(raw_notifier_call_chain);
@@ -475,6 +531,8 @@ EXPORT_SYMBOL_GPL(srcu_notifier_chain_un
  *	@nh: Pointer to head of the SRCU 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.
+ *	@nr_calls: See comment for notifier_call_chain
  *
  *	Calls each function in a notifier chain in turn.  The functions
  *	run in a process context, so they are allowed to block.
@@ -487,18 +545,25 @@ EXPORT_SYMBOL_GPL(srcu_notifier_chain_un
  *	of the last notifier function called.
  */
 
-int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
-		unsigned long val, void *v)
+int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
+			       unsigned long val, void *v,
+			       int nr_to_call, unsigned int *nr_calls)
 {
 	int ret;
 	int idx;
 
 	idx = srcu_read_lock(&nh->srcu);
-	ret = notifier_call_chain(&nh->head, val, v);
+	ret = notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
 	srcu_read_unlock(&nh->srcu, idx);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(__srcu_notifier_call_chain);
 
+int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
+		unsigned long val, void *v)
+{
+	return __srcu_notifier_call_chain(nh, val, v, -1, NULL);
+}
 EXPORT_SYMBOL_GPL(srcu_notifier_call_chain);
 
 /**
Index: hotplug/include/linux/notifier.h
===================================================================
--- hotplug.orig/include/linux/notifier.h
+++ hotplug/include/linux/notifier.h
@@ -132,12 +132,20 @@ extern int srcu_notifier_chain_unregiste
 
 extern int atomic_notifier_call_chain(struct atomic_notifier_head *,
 		unsigned long val, void *v);
+extern int __atomic_notifier_call_chain(struct atomic_notifier_head *,
+	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
 extern int blocking_notifier_call_chain(struct blocking_notifier_head *,
 		unsigned long val, void *v);
+extern int __blocking_notifier_call_chain(struct blocking_notifier_head *,
+	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
 extern int raw_notifier_call_chain(struct raw_notifier_head *,
 		unsigned long val, void *v);
+extern int __raw_notifier_call_chain(struct raw_notifier_head *,
+	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
 extern int srcu_notifier_call_chain(struct srcu_notifier_head *,
 		unsigned long val, void *v);
+extern int __srcu_notifier_call_chain(struct srcu_notifier_head *,
+	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
 
 #define NOTIFY_DONE		0x0000		/* Don't care */
 #define NOTIFY_OK		0x0001		/* Suits me */
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

[PATCH 2/4] Define and use new events,CPU_LOCK_ACQUIRE and CPU_LOCK_RELEASE.

[Gautham R Shenoy]

This is an attempt to provide an alternate mechanism for postponing
a hotplug event instead of using a global mechanism like lock_cpu_hotplug.

The proposal is to add two new events namely CPU_LOCK_ACQUIRE and
CPU_LOCK_RELEASE. The notification for these two events would be sent
out before and after a cpu_hotplug event respectively.

During the CPU_LOCK_ACQUIRE event, a cpu-hotplug-aware subsystem is
supposed to acquire any per-subsystem hotcpu mutex ( Eg. workqueue_mutex
in kernel/workqueue.c ). 

During the CPU_LOCK_RELEASE release event the cpu-hotplug-aware subsystem
is supposed to release the per-subsystem hotcpu mutex.

The reasons for defining new events as opposed to reusing the existing events
like CPU_UP_PREPARE/CPU_UP_FAILED/CPU_ONLINE for locking/unlocking of 
per-subsystem hotcpu mutexes are as follow:

	- CPU_LOCK_ACQUIRE: All hotcpu mutexes are taken before subsystems
	start handling pre-hotplug events like CPU_UP_PREPARE/CPU_DOWN_PREPARE 
	etc, thus ensuring a clean handling of these events. 
	
	- CPU_LOCK_RELEASE: The hotcpu mutexes will be released only after
	all subsystems have handled post-hotplug events like CPU_DOWN_FAILED,
	CPU_DEAD,CPU_ONLINE etc thereby ensuring that there are no subsequent
	clashes amongst the interdependent subsystems after a cpu hotplugs.
	
This patch also uses __raw_notifier_call chain in _cpu_up to take care 
of the dependency between the two consequetive calls to
raw_notifier_call_chain.

Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>

--
 include/linux/notifier.h |    2 ++
 kernel/cpu.c             |   15 +++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

Index: hotplug/kernel/cpu.c
===================================================================
--- hotplug.orig/kernel/cpu.c
+++ hotplug/kernel/cpu.c
@@ -132,6 +132,8 @@ static int _cpu_down(unsigned int cpu)
 	if (!cpu_online(cpu))
 		return -EINVAL;
 
+	raw_notifier_call_chain(&cpu_chain, CPU_LOCK_ACQUIRE,
+						(void *)(long)cpu);
 	err = raw_notifier_call_chain(&cpu_chain, CPU_DOWN_PREPARE,
 						(void *)(long)cpu);
 	if (err == NOTIFY_BAD) {
@@ -185,6 +187,8 @@ out_thread:
 	err = kthread_stop(p);
 out_allowed:
 	set_cpus_allowed(current, old_allowed);
+	raw_notifier_call_chain(&cpu_chain, CPU_LOCK_RELEASE,
+						(void *)(long)cpu);
 	return err;
 }
 
@@ -206,13 +210,15 @@ int cpu_down(unsigned int cpu)
 /* Requires cpu_add_remove_lock to be held */
 static int __devinit _cpu_up(unsigned int cpu)
 {
-	int ret;
+	int ret, nr_calls = 0;
 	void *hcpu = (void *)(long)cpu;
 
 	if (cpu_online(cpu) || !cpu_present(cpu))
 		return -EINVAL;
 
-	ret = raw_notifier_call_chain(&cpu_chain, CPU_UP_PREPARE, hcpu);
+	raw_notifier_call_chain(&cpu_chain, CPU_LOCK_ACQUIRE, hcpu);
+	ret = __raw_notifier_call_chain(&cpu_chain, CPU_UP_PREPARE, hcpu,
+							-1, &nr_calls);
 	if (ret == NOTIFY_BAD) {
 		printk("%s: attempt to bring up CPU %u failed\n",
 				__FUNCTION__, cpu);
@@ -233,8 +239,9 @@ static int __devinit _cpu_up(unsigned in
 
 out_notify:
 	if (ret != 0)
-		raw_notifier_call_chain(&cpu_chain,
-				CPU_UP_CANCELED, hcpu);
+		__raw_notifier_call_chain(&cpu_chain,
+				CPU_UP_CANCELED, hcpu, nr_calls, NULL);
+	raw_notifier_call_chain(&cpu_chain, CPU_LOCK_RELEASE, hcpu);
 
 	return ret;
 }
Index: hotplug/include/linux/notifier.h
===================================================================
--- hotplug.orig/include/linux/notifier.h
+++ hotplug/include/linux/notifier.h
@@ -194,6 +194,8 @@ extern int __srcu_notifier_call_chain(st
 #define CPU_DOWN_PREPARE	0x0005 /* CPU (unsigned)v going down */
 #define CPU_DOWN_FAILED		0x0006 /* CPU (unsigned)v NOT going down */
 #define CPU_DEAD		0x0007 /* CPU (unsigned)v dead */
+#define CPU_LOCK_ACQUIRE	0x0008 /* Acquire all hotcpu locks */
+#define CPU_LOCK_RELEASE	0x0009 /* Release all hotcpu locks */
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_NOTIFIER_H */
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

[PATCH 3/4] Eliminate lock_cpu_hotplug in kernel/sched.c

[Gautham R Shenoy]

Eliminate lock_cpu_hotplug from kernel/sched.c and use sched_hotcpu_mutex
instead to postpone a hotplug event. 

In the migration_call hotcpu callback function, take sched_hotcpu_mutex
while handling the event CPU_LOCK_ACQUIRE and release it while handling
CPU_LOCK_RELEASE event.

Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>

--
 kernel/sched.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

Index: hotplug/kernel/sched.c
===================================================================
--- hotplug.orig/kernel/sched.c
+++ hotplug/kernel/sched.c
@@ -267,6 +267,7 @@ struct rq {
 };
 
 static DEFINE_PER_CPU(struct rq, runqueues);
+static DEFINE_MUTEX(sched_hotcpu_mutex);
 
 static inline int cpu_of(struct rq *rq)
 {
@@ -4327,13 +4328,13 @@ long sched_setaffinity(pid_t pid, cpumas
 	struct task_struct *p;
 	int retval;
 
-	lock_cpu_hotplug();
+	mutex_lock(&sched_hotcpu_mutex);
 	read_lock(&tasklist_lock);
 
 	p = find_process_by_pid(pid);
 	if (!p) {
 		read_unlock(&tasklist_lock);
-		unlock_cpu_hotplug();
+		mutex_unlock(&sched_hotcpu_mutex);
 		return -ESRCH;
 	}
 
@@ -4360,7 +4361,7 @@ long sched_setaffinity(pid_t pid, cpumas
 
 out_unlock:
 	put_task_struct(p);
-	unlock_cpu_hotplug();
+	mutex_unlock(&sched_hotcpu_mutex);
 	return retval;
 }
 
@@ -4417,7 +4418,7 @@ long sched_getaffinity(pid_t pid, cpumas
 	struct task_struct *p;
 	int retval;
 
-	lock_cpu_hotplug();
+	mutex_lock(&sched_hotcpu_mutex);
 	read_lock(&tasklist_lock);
 
 	retval = -ESRCH;
@@ -4433,7 +4434,7 @@ long sched_getaffinity(pid_t pid, cpumas
 
 out_unlock:
 	read_unlock(&tasklist_lock);
-	unlock_cpu_hotplug();
+	mutex_unlock(&sched_hotcpu_mutex);
 	if (retval)
 		return retval;
 
@@ -5225,6 +5226,10 @@ migration_call(struct notifier_block *nf
 	struct rq *rq;
 
 	switch (action) {
+	case CPU_LOCK_ACQUIRE:
+		mutex_lock(&sched_hotcpu_mutex);
+		break;
+
 	case CPU_UP_PREPARE:
 		p = kthread_create(migration_thread, hcpu, "migration/%d",cpu);
 		if (IS_ERR(p))
@@ -5270,7 +5275,7 @@ migration_call(struct notifier_block *nf
 		BUG_ON(rq->nr_running != 0);
 
 		/* No need to migrate the tasks: it was best-effort if
-		 * they didn't do lock_cpu_hotplug().  Just wake up
+		 * they didn't take sched_hotcpu_mutex.  Just wake up
 		 * the requestors. */
 		spin_lock_irq(&rq->lock);
 		while (!list_empty(&rq->migration_queue)) {
@@ -5283,6 +5288,10 @@ migration_call(struct notifier_block *nf
 		}
 		spin_unlock_irq(&rq->lock);
 		break;
+
+	case CPU_LOCK_RELEASE:
+		mutex_unlock(&sched_hotcpu_mutex);
+		break;
 #endif
 	}
 	return NOTIFY_OK;
@@ -6652,10 +6661,10 @@ int arch_reinit_sched_domains(void)
 {
 	int err;
 
-	lock_cpu_hotplug();
+	mutex_lock(&sched_hotcpu_mutex);
 	detach_destroy_domains(&cpu_online_map);
 	err = arch_init_sched_domains(&cpu_online_map);
-	unlock_cpu_hotplug();
+	mutex_unlock(&sched_hotcpu_mutex);
 
 	return err;
 }
@@ -6763,12 +6772,12 @@ void __init sched_init_smp(void)
 {
 	cpumask_t non_isolated_cpus;
 
-	lock_cpu_hotplug();
+	mutex_lock(&sched_hotcpu_mutex);
 	arch_init_sched_domains(&cpu_online_map);
 	cpus_andnot(non_isolated_cpus, cpu_online_map, cpu_isolated_map);
 	if (cpus_empty(non_isolated_cpus))
 		cpu_set(smp_processor_id(), non_isolated_cpus);
-	unlock_cpu_hotplug();
+	mutex_unlock(&sched_hotcpu_mutex);
 	/* XXX: Theoretical race here - CPU may be hotplugged now */
 	hotcpu_notifier(update_sched_domains, 0);
 
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

[PATCH 4/4] Handle CPU_LOCK_ACQUIRE and CPU_LOCK_RELEASE in workqueue_cpu_callback.

[Gautham R Shenoy]

In hot-cpu callback function workqueue_cpu_callback, lock the workqueue_mutex 
under CPU_LOCK_ACQUIRE and release it under CPU_LOCK_RELEASE.

This eliminates handling of redundant events namely CPU_DOWN_PREPARE and
CPU_DOWN_FAILED.

Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>

--
 kernel/workqueue.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

Index: hotplug/kernel/workqueue.c
===================================================================
--- hotplug.orig/kernel/workqueue.c
+++ hotplug/kernel/workqueue.c
@@ -638,8 +638,11 @@ static int __devinit workqueue_cpu_callb
 	struct workqueue_struct *wq;
 
 	switch (action) {
-	case CPU_UP_PREPARE:
+	case CPU_LOCK_ACQUIRE:
 		mutex_lock(&workqueue_mutex);
+		break;
+
+	case CPU_UP_PREPARE:
 		/* Create a new workqueue thread for it. */
 		list_for_each_entry(wq, &workqueues, list) {
 			if (!create_workqueue_thread(wq, hotcpu)) {
@@ -658,7 +661,6 @@ static int __devinit workqueue_cpu_callb
 			kthread_bind(cwq->thread, hotcpu);
 			wake_up_process(cwq->thread);
 		}
-		mutex_unlock(&workqueue_mutex);
 		break;
 
 	case CPU_UP_CANCELED:
@@ -670,15 +672,6 @@ static int __devinit workqueue_cpu_callb
 				     any_online_cpu(cpu_online_map));
 			cleanup_workqueue_thread(wq, hotcpu);
 		}
-		mutex_unlock(&workqueue_mutex);
-		break;
-
-	case CPU_DOWN_PREPARE:
-		mutex_lock(&workqueue_mutex);
-		break;
-
-	case CPU_DOWN_FAILED:
-		mutex_unlock(&workqueue_mutex);
 		break;
 
 	case CPU_DEAD:
@@ -686,6 +679,9 @@ static int __devinit workqueue_cpu_callb
 			cleanup_workqueue_thread(wq, hotcpu);
 		list_for_each_entry(wq, &workqueues, list)
 			take_over_work(wq, hotcpu);
+		break;
+
+	case CPU_LOCK_RELEASE:
 		mutex_unlock(&workqueue_mutex);
 		break;
 	}
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

Re: [PATCH 1/4] Extend notifier_call_chain to count nr_calls made.

[Randy Dunlap]

On Tue, 14 Nov 2006 17:50:51 +0530 Gautham R Shenoy wrote:

> Provide notifier_call_chain with an option to call only a specified number of 
> notifiers and also record the number of call to notifiers made.
> 
> The need for this enhancement was identified in the post entitled 
> "Slab - Eliminate lock_cpu_hotplug from slab" 
> (http://lkml.org/lkml/2006/10/28/92) by Ravikiran G Thirumalai and 
> Andrew Morton.
> 
> This patch adds two additional parameters to notifier_call_chain API namely 
>  - int nr_to_calls : Number of notifier_functions to be called. 
>  		     The don't care value is -1.
> 
>  - unsigned int *nr_calls : Records the total number of notifier_funtions 
> 			    called by notifier_call_chain. The don't care
> 			    value is NULL.

Those could (should?) be the same data type.

> Credit : Andrew Morton <akpm@osdl.org> 
> Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
> 
> --
>  include/linux/notifier.h |    8 +++
>  kernel/sys.c             |   97 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 89 insertions(+), 16 deletions(-)
> 
> Index: hotplug/kernel/sys.c
> ===================================================================
> --- hotplug.orig/kernel/sys.c
> +++ hotplug/kernel/sys.c
> @@ -134,19 +134,41 @@ static int notifier_chain_unregister(str
>  	return -ENOENT;
>  }
>  
> +/*
> + * notifier_call_chain - Informs the registered notifiers about an event.
> + *
> + *	@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.
> + *
> + * 	RETURN VALUE:	notifier_call_chain returns the value returned by the
> + *			last notifier function called.
> + */

You can make that comment block be kernel-doc format by using
/**
as the comment introduction and removing the blank line after the
function name & short description.

>  static int __kprobes notifier_call_chain(struct notifier_block **nl,
> -		unsigned long val, void *v)
> +					unsigned long val, void *v,
> +					int nr_to_call,	unsigned int *nr_calls)
>  {
>  	int ret = NOTIFY_DONE;
>  	struct notifier_block *nb, *next_nb;
...
>  }
> @@ -205,10 +227,13 @@ int atomic_notifier_chain_unregister(str
>  EXPORT_SYMBOL_GPL(atomic_notifier_chain_unregister);
>  
>  /**
> - *	atomic_notifier_call_chain - Call functions in an atomic notifier chain
> + *	__atomic_notifier_call_chain - Call functions in an atomic notifier
> + *				       chain

Don't break the short function description line; kernel-doc does not
support that.

>   *	@nh: Pointer to head of the atomic notifier chain
>   *	@val: Value passed unmodified to notifier function
>   *	@v: Pointer passed unmodified to notifier function
> + *	@nr_to_call: See the comment for notifier_call_chain.
> + *	@nr_calls: See the comment for notifier_call_chain.
>   *
>   *	Calls each function in a notifier chain in turn.  The functions
>   *	run in an atomic context, so they must not block.
> @@ -222,19 +247,27 @@ EXPORT_SYMBOL_GPL(atomic_notifier_chain_
>   *	of the last notifier function called.
>   */
>   
> -int __kprobes atomic_notifier_call_chain(struct atomic_notifier_head *nh,
> -		unsigned long val, void *v)
> +int __kprobes __atomic_notifier_call_chain(struct atomic_notifier_head *nh,
> +					unsigned long val, void *v,
> +					int nr_to_call, unsigned int *nr_calls)
>  {
...
>  }
>  
> -EXPORT_SYMBOL_GPL(atomic_notifier_call_chain);
> +EXPORT_SYMBOL_GPL(__atomic_notifier_call_chain);
>  
> +int __kprobes atomic_notifier_call_chain(struct atomic_notifier_head *nh,
> +		unsigned long val, void *v)
> +{
> +	return __atomic_notifier_call_chain(nh, val, v, -1, NULL);
> +}
> +
> +EXPORT_SYMBOL_GPL(atomic_notifier_call_chain);
>  /*
>   *	Blocking notifier chain routines.  All access to the chain is
>   *	synchronized by an rwsem.
> @@ -304,10 +337,13 @@ int blocking_notifier_chain_unregister(s
>  EXPORT_SYMBOL_GPL(blocking_notifier_chain_unregister);
>  
>  /**
> - *	blocking_notifier_call_chain - Call functions in a blocking notifier chain
> + *	__blocking_notifier_call_chain - Call functions in a blocking notifier
> + *					 chain

kernel-doc requires that the function description fit on one source line,
so don't break it (even if it is > 80 columns; yes, I know it needs to be
fixed)

>   *	@nh: 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: See comment for notifier_call_chain.
> + *	@nr_calls: See comment for notifier_call_chain.
>   *
>   *	Calls each function in a notifier chain in turn.  The functions
>   *	run in a process context, so they are allowed to block.

> Index: hotplug/include/linux/notifier.h
> ===================================================================
> --- hotplug.orig/include/linux/notifier.h
> +++ hotplug/include/linux/notifier.h
> @@ -132,12 +132,20 @@ extern int srcu_notifier_chain_unregiste
>  
>  extern int atomic_notifier_call_chain(struct atomic_notifier_head *,
>  		unsigned long val, void *v);
> +extern int __atomic_notifier_call_chain(struct atomic_notifier_head *,

While you are changing these lines, please put a prototype parameter
name for all parameters; i.e., add something like "notifier" 
or "nh" after the '*' on all of these.

> +	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
>  extern int blocking_notifier_call_chain(struct blocking_notifier_head *,
>  		unsigned long val, void *v);
> +extern int __blocking_notifier_call_chain(struct blocking_notifier_head *,
> +	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
>  extern int raw_notifier_call_chain(struct raw_notifier_head *,
>  		unsigned long val, void *v);
> +extern int __raw_notifier_call_chain(struct raw_notifier_head *,
> +	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
>  extern int srcu_notifier_call_chain(struct srcu_notifier_head *,
>  		unsigned long val, void *v);
> +extern int __srcu_notifier_call_chain(struct srcu_notifier_head *,
> +	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
>  
>  #define NOTIFY_DONE		0x0000		/* Don't care */
>  #define NOTIFY_OK		0x0001		/* Suits me */
> -- 

---
~Randy

Re: [PATCH 1/4] Extend notifier_call_chain to count nr_calls made.

[Gautham R Shenoy]

On Tue, Nov 14, 2006 at 10:18:06AM -0800, Randy Dunlap wrote:
> On Tue, 14 Nov 2006 17:50:51 +0530 Gautham R Shenoy wrote:
> 
> > Provide notifier_call_chain with an option to call only a specified number of 
> > notifiers and also record the number of call to notifiers made.
> > 
> > The need for this enhancement was identified in the post entitled 
> > "Slab - Eliminate lock_cpu_hotplug from slab" 
> > (http://lkml.org/lkml/2006/10/28/92) by Ravikiran G Thirumalai and 
> > Andrew Morton.
> > 
> > This patch adds two additional parameters to notifier_call_chain API namely 
> >  - int nr_to_calls : Number of notifier_functions to be called. 
> >  		     The don't care value is -1.
> > 
> >  - unsigned int *nr_calls : Records the total number of notifier_funtions 
> > 			    called by notifier_call_chain. The don't care
> > 			    value is NULL.
> 
> Those could (should?) be the same data type.

Whoops! Yes, they should be the same. 
I was trying to solve this problem with only one parameter 
(which we can, but the signature would look very confusing) and was
using the unsigned int* there. Will change it to int *. 
Thanks for pointing that out.
> 
> > Credit : Andrew Morton <akpm@osdl.org> 
> > Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
> > 
> > --
> >  include/linux/notifier.h |    8 +++
> >  kernel/sys.c             |   97 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 89 insertions(+), 16 deletions(-)
> > 
> > Index: hotplug/kernel/sys.c
> > ===================================================================
> > --- hotplug.orig/kernel/sys.c
> > +++ hotplug/kernel/sys.c
> > @@ -134,19 +134,41 @@ static int notifier_chain_unregister(str
> >  	return -ENOENT;
> >  }
> >  
> > +/*
> > + * notifier_call_chain - Informs the registered notifiers about an event.
> > + *
> > + *	@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.
> > + *
> > + * 	RETURN VALUE:	notifier_call_chain returns the value returned by the
> > + *			last notifier function called.
> > + */
> 
> You can make that comment block be kernel-doc format by using
> /**
> as the comment introduction and removing the blank line after the
> function name & short description.

Will do that. But out of curiousity, do the comments of even static functions
get reflected in kernel doc ? :?

> 
> >  static int __kprobes notifier_call_chain(struct notifier_block **nl,
> > -		unsigned long val, void *v)
> > +					unsigned long val, void *v,
> > +					int nr_to_call,	unsigned int *nr_calls)
> >  {
> >  	int ret = NOTIFY_DONE;
> >  	struct notifier_block *nb, *next_nb;
> ...
> >  }
> > @@ -205,10 +227,13 @@ int atomic_notifier_chain_unregister(str
> >  EXPORT_SYMBOL_GPL(atomic_notifier_chain_unregister);
> >  
> >  /**
> > - *	atomic_notifier_call_chain - Call functions in an atomic notifier chain
> > + *	__atomic_notifier_call_chain - Call functions in an atomic notifier
> > + *				       chain
> 
> Don't break the short function description line; kernel-doc does not
> support that.

Ok. Didn't know that. Will correct it.

> 
> >   *	@nh: Pointer to head of the atomic notifier chain

[snip!]

> > Index: hotplug/include/linux/notifier.h
> > ===================================================================
> > --- hotplug.orig/include/linux/notifier.h
> > +++ hotplug/include/linux/notifier.h
> > @@ -132,12 +132,20 @@ extern int srcu_notifier_chain_unregiste
> >  
> >  extern int atomic_notifier_call_chain(struct atomic_notifier_head *,
> >  		unsigned long val, void *v);
> > +extern int __atomic_notifier_call_chain(struct atomic_notifier_head *,
> 
> While you are changing these lines, please put a prototype parameter
> name for all parameters; i.e., add something like "notifier" 
> or "nh" after the '*' on all of these.
>

Yup. Will do.

 
> ---
> ~Randy

thanks,
gautham.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

Re: [PATCH 1/4] Extend notifier_call_chain to count nr_calls made.

[Randy Dunlap]

Gautham R Shenoy wrote:
> On Tue, Nov 14, 2006 at 10:18:06AM -0800, Randy Dunlap wrote:
>> On Tue, 14 Nov 2006 17:50:51 +0530 Gautham R Shenoy wrote:
>>
>>>  include/linux/notifier.h |    8 +++
>>>  kernel/sys.c             |   97 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 89 insertions(+), 16 deletions(-)
>>>
>>> Index: hotplug/kernel/sys.c
>>> ===================================================================
>>> --- hotplug.orig/kernel/sys.c
>>> +++ hotplug/kernel/sys.c
>>> @@ -134,19 +134,41 @@ static int notifier_chain_unregister(str
>>>  	return -ENOENT;
>>>  }
>>>  
>>> +/*
>>> + * notifier_call_chain - Informs the registered notifiers about an event.
>>> + *
>>> + *	@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.
>>> + *
>>> + * 	RETURN VALUE:	notifier_call_chain returns the value returned by the
>>> + *			last notifier function called.
>>> + */
>> You can make that comment block be kernel-doc format by using
>> /**
>> as the comment introduction and removing the blank line after the
>> function name & short description.
> 
> Will do that. But out of curiousity, do the comments of even static functions
> get reflected in kernel doc ? :?

They can be, but static functions are low priority for kernel-doc
IMO, so it's not a big deal to me if you don't make that be kernel-doc.

>>>  static int __kprobes notifier_call_chain(struct notifier_block **nl,
>>> -		unsigned long val, void *v)
>>> +					unsigned long val, void *v,
>>> +					int nr_to_call,	unsigned int *nr_calls)
>>>  {
>>>  	int ret = NOTIFY_DONE;
>>>  	struct notifier_block *nb, *next_nb;
>> ...
>>>  }

Thanks,
-- 
~Randy

[PATCH 1-fix/4] Fix extend notifier_call_chain to count nr_calls made.

[Gautham R Shenoy]

Hi Andrew,

Please apply the following patch on top of 
extend-notifier_call_chain-to-count-nr_calls-made.patch.
This patch incorporates the Randy's suggestions.

thanks,
gautham.


This patch

* Corrects the type of nr_calls to int * from unsigned int * in
notifier_call_chain and it's subsequent callers.

* Converts comments of notifier_call_chain to be compliant with kernel-docs
standards.

*Reverts the changes made to the comments of other *_notifier_call_chain.

*Adds parameter names to a few functions prototypes in
include/linux/notifier.h .

Depends on patch: extend-notifier_call_chain-to-count-nr_calls-made.patch


Cc: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>

--
 include/linux/notifier.h |   58 +++++++++++++++++++++++------------------------
 kernel/sys.c             |   24 ++++++++-----------
 2 files changed, 39 insertions(+), 43 deletions(-)

Index: hotplug/kernel/sys.c
===================================================================
--- hotplug.orig/kernel/sys.c
+++ hotplug/kernel/sys.c
@@ -134,9 +134,8 @@ static int notifier_chain_unregister(str
 	return -ENOENT;
 }
 
-/*
+/**
  * notifier_call_chain - Informs the registered notifiers about an event.
- *
  *	@nl:		Pointer to head of the blocking notifier chain
  *	@val:		Value passed unmodified to notifier function
  *	@v:		Pointer passed unmodified to notifier function
@@ -144,14 +143,13 @@ static int notifier_chain_unregister(str
  *		     	value of this parameter is -1.
  *	@nr_calls:	Records the number of notifications sent. Don't care
  *		   	value of this field is NULL.
- *
- * 	RETURN VALUE:	notifier_call_chain returns the value returned by the
+ * 	@returns:	notifier_call_chain returns the value returned by the
  *			last notifier function called.
  */
 
 static int __kprobes notifier_call_chain(struct notifier_block **nl,
 					unsigned long val, void *v,
-					int nr_to_call,	unsigned int *nr_calls)
+					int nr_to_call,	int *nr_calls)
 {
 	int ret = NOTIFY_DONE;
 	struct notifier_block *nb, *next_nb;
@@ -227,8 +225,7 @@ int atomic_notifier_chain_unregister(str
 EXPORT_SYMBOL_GPL(atomic_notifier_chain_unregister);
 
 /**
- *	__atomic_notifier_call_chain - Call functions in an atomic notifier
- *				       chain
+ *	__atomic_notifier_call_chain - Call functions in an atomic notifier chain
  *	@nh: Pointer to head of the atomic notifier chain
  *	@val: Value passed unmodified to notifier function
  *	@v: Pointer passed unmodified to notifier function
@@ -249,7 +246,7 @@ EXPORT_SYMBOL_GPL(atomic_notifier_chain_
  
 int __kprobes __atomic_notifier_call_chain(struct atomic_notifier_head *nh,
 					unsigned long val, void *v,
-					int nr_to_call, unsigned int *nr_calls)
+					int nr_to_call, int *nr_calls)
 {
 	int ret;
 
@@ -337,8 +334,7 @@ int blocking_notifier_chain_unregister(s
 EXPORT_SYMBOL_GPL(blocking_notifier_chain_unregister);
 
 /**
- *	__blocking_notifier_call_chain - Call functions in a blocking notifier
- *					 chain
+ *	__blocking_notifier_call_chain - Call functions in a blocking notifier chain
  *	@nh: Pointer to head of the blocking notifier chain
  *	@val: Value passed unmodified to notifier function
  *	@v: Pointer passed unmodified to notifier function
@@ -358,7 +354,7 @@ EXPORT_SYMBOL_GPL(blocking_notifier_chai
  
 int __blocking_notifier_call_chain(struct blocking_notifier_head *nh,
 				   unsigned long val, void *v,
-				   int nr_to_call, unsigned int * nr_calls)
+				   int nr_to_call, int *nr_calls)
 {
 	int ret;
 
@@ -442,7 +438,7 @@ EXPORT_SYMBOL_GPL(raw_notifier_chain_unr
 
 int __raw_notifier_call_chain(struct raw_notifier_head *nh,
 			      unsigned long val, void *v,
-			      int nr_to_call, unsigned int *nr_calls)
+			      int nr_to_call, int *nr_calls)
 {
 	return notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
 }
@@ -527,7 +523,7 @@ int srcu_notifier_chain_unregister(struc
 EXPORT_SYMBOL_GPL(srcu_notifier_chain_unregister);
 
 /**
- *	srcu_notifier_call_chain - Call functions in an SRCU notifier chain
+ *	__srcu_notifier_call_chain - Call functions in an SRCU notifier chain
  *	@nh: Pointer to head of the SRCU notifier chain
  *	@val: Value passed unmodified to notifier function
  *	@v: Pointer passed unmodified to notifier function
@@ -547,7 +543,7 @@ EXPORT_SYMBOL_GPL(srcu_notifier_chain_un
 
 int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
 			       unsigned long val, void *v,
-			       int nr_to_call, unsigned int *nr_calls)
+			       int nr_to_call, int *nr_calls)
 {
 	int ret;
 	int idx;
Index: hotplug/include/linux/notifier.h
===================================================================
--- hotplug.orig/include/linux/notifier.h
+++ hotplug/include/linux/notifier.h
@@ -112,40 +112,40 @@ extern void srcu_init_notifier_head(stru
 
 #ifdef __KERNEL__
 
-extern int atomic_notifier_chain_register(struct atomic_notifier_head *,
-		struct notifier_block *);
-extern int blocking_notifier_chain_register(struct blocking_notifier_head *,
-		struct notifier_block *);
-extern int raw_notifier_chain_register(struct raw_notifier_head *,
-		struct notifier_block *);
-extern int srcu_notifier_chain_register(struct srcu_notifier_head *,
-		struct notifier_block *);
-
-extern int atomic_notifier_chain_unregister(struct atomic_notifier_head *,
-		struct notifier_block *);
-extern int blocking_notifier_chain_unregister(struct blocking_notifier_head *,
-		struct notifier_block *);
-extern int raw_notifier_chain_unregister(struct raw_notifier_head *,
-		struct notifier_block *);
-extern int srcu_notifier_chain_unregister(struct srcu_notifier_head *,
-		struct notifier_block *);
+extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
+		struct notifier_block *nb);
+extern int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
+		struct notifier_block *nb);
+extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
+		struct notifier_block *nb);
+extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
+		struct notifier_block *nb);
+
+extern int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
+		struct notifier_block *nb);
+extern int blocking_notifier_chain_unregister(struct blocking_notifier_head *nh,
+		struct notifier_block *nb);
+extern int raw_notifier_chain_unregister(struct raw_notifier_head *nh,
+		struct notifier_block *nb);
+extern int srcu_notifier_chain_unregister(struct srcu_notifier_head *nh,
+		struct notifier_block *nb);
 
-extern int atomic_notifier_call_chain(struct atomic_notifier_head *,
+extern int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
 		unsigned long val, void *v);
-extern int __atomic_notifier_call_chain(struct atomic_notifier_head *,
-	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
-extern int blocking_notifier_call_chain(struct blocking_notifier_head *,
+extern int __atomic_notifier_call_chain(struct atomic_notifier_head *nh,
+	unsigned long val, void *v, int nr_to_call, int *nr_calls);
+extern int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
 		unsigned long val, void *v);
-extern int __blocking_notifier_call_chain(struct blocking_notifier_head *,
-	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
-extern int raw_notifier_call_chain(struct raw_notifier_head *,
+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(struct raw_notifier_head *,
-	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
-extern int srcu_notifier_call_chain(struct srcu_notifier_head *,
+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 srcu_notifier_call_chain(struct srcu_notifier_head *nh,
 		unsigned long val, void *v);
-extern int __srcu_notifier_call_chain(struct srcu_notifier_head *,
-	unsigned long val, void *v, int nr_to_call, unsigned int *nr_calls);
+extern int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
+	unsigned long val, void *v, int nr_to_call, int *nr_calls);
 
 #define NOTIFY_DONE		0x0000		/* Don't care */
 #define NOTIFY_OK		0x0001		/* Suits me */
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

Re: [PATCH 1/4] Extend notifier_call_chain to count nr_calls made.

[Andrew Morton]

On Tue, 14 Nov 2006 17:50:51 +0530
Gautham R Shenoy <ego@in.ibm.com> wrote:

> Provide notifier_call_chain with an option to call only a specified number of 
> notifiers and also record the number of call to notifiers made.
> 
> The need for this enhancement was identified in the post entitled 
> "Slab - Eliminate lock_cpu_hotplug from slab" 
> (http://lkml.org/lkml/2006/10/28/92) by Ravikiran G Thirumalai and 
> Andrew Morton.
> 
> This patch adds two additional parameters to notifier_call_chain API namely 
>  - int nr_to_calls : Number of notifier_functions to be called. 
>  		     The don't care value is -1.
> 
>  - unsigned int *nr_calls : Records the total number of notifier_funtions 
> 			    called by notifier_call_chain. The don't care
> 			    value is NULL.
> 
> ...
>
> +
>  static int __kprobes notifier_call_chain(struct notifier_block **nl,
> -		unsigned long val, void *v)
> +					unsigned long val, void *v,
> +					int nr_to_call,	unsigned int *nr_calls)
>  {
>  	int ret = NOTIFY_DONE;
>  	struct notifier_block *nb, *next_nb;
>  
>  	nb = rcu_dereference(*nl);
> -	while (nb) {
> +
> +	while (nb && nr_to_call) {
>  		next_nb = rcu_dereference(nb->next);
>  		ret = nb->notifier_call(nb, val, v);
> +
> +		if (nr_calls)
> +			*nr_calls ++;

This gets

kernel/sys.c: In function 'notifier_call_chain':
kernel/sys.c:164: warning: value computed is not used


And indeed, this code doesn't work.

What happened?

Re: [PATCH 1/4] Extend notifier_call_chain to count nr_calls made.

[Gautham R Shenoy]

Hi Andrew,
	
On Mon, Nov 20, 2006 at 10:19:41PM -0800, Andrew Morton wrote:

> > +
> > +		if (nr_calls)
> > +			*nr_calls ++;
> 
> This gets
> 
> kernel/sys.c: In function 'notifier_call_chain':
> kernel/sys.c:164: warning: value computed is not used
> 
> 
> And indeed, this code doesn't work.
> 
> What happened?

I didn't get the warnings because my test box still has the prehistoric
version 3.4.4 of gcc. 
I compiled the code with gcc 4.1.1 and got the warnings.

The code does not work because of my carelessness. 
It should have been (*nr_calls)++ in the first place. I apologise.

Thanks for fixing it.

Regards
gautham.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

Re: [RFC 0/4] Cpu-Hotplug: Use per subsystem hot-cpu mutexes.

[Andrew Morton]

On Tue, 14 Nov 2006 17:48:32 +0530
Gautham R Shenoy <ego@in.ibm.com> wrote:

> Since 2.6.18-something, the community has been bugged by the problem to
> provide a clean and a stable mechanism to postpone a cpu-hotplug event
> as lock_cpu_hotplug was badly broken.
> 
> This is another proposal towards solving that problem. This one is 
> along the lines of the solution provided in kernel/workqueue.c

The approach seems sane to me.  Sort-of direct, specific and transactional..

I applied this fixup:

diff -puN kernel/cpu.c~define-and-use-new-eventscpu_lock_acquire-and-cpu_lock_release-fix kernel/cpu.c
--- a/kernel/cpu.c~define-and-use-new-eventscpu_lock_acquire-and-cpu_lock_release-fix
+++ a/kernel/cpu.c
@@ -139,7 +139,8 @@ static int _cpu_down(unsigned int cpu)
 	if (err == NOTIFY_BAD) {
 		printk("%s: attempt to take down CPU %u failed\n",
 				__FUNCTION__, cpu);
-		return -EINVAL;
+		err = -EINVAL;
+		goto out_release;
 	}
 
 	/* Ensure that we are not runnable on dying cpu */
@@ -187,6 +188,7 @@ out_thread:
 	err = kthread_stop(p);
 out_allowed:
 	set_cpus_allowed(current, old_allowed);
+out_release:
 	raw_notifier_call_chain(&cpu_chain, CPU_LOCK_RELEASE,
 						(void *)(long)cpu);
 	return err;
_


please send a patch to fix up the kerneldoc things which Randy spotted,
thanks.