[PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

Hi,

While working on the IO CPU affinity bits, I had to convert some more
archs to support smp_call_function_single(). Nicks original patch
for x86-64 also included improvements for smp_call_function(), so
it isn't serialized by call_lock anymore (but still hits call_lock
on every call). I've made further improvements and changes upon that
while converting x86, going further and updating IA64 and PPC made me
realize that we have a lot of shared code for the same functionality.

So I came up with kernel/smp.c that basically just holds the new
and abstracted code for issuing smp_call_function() and
smp_call_function_single() along with the helpers and interrupt
handlers associated. Perhaps a better place would be lib/smp.c, I'm
open to suggestions.

Even with half of this being new functionality, the diffstat still looks
pretty nice:

 arch/ia64/kernel/smp.c                     |  171 ++---------
 arch/powerpc/kernel/smp.c                  |  223 +-------------
 arch/powerpc/platforms/cell/interrupt.c    |    1 
 arch/powerpc/platforms/ps3/smp.c           |    7 
 arch/powerpc/platforms/pseries/xics.c      |    6 
 arch/powerpc/sysdev/mpic.c                 |    2 
 arch/x86/kernel/entry_64.S                 |    3 
 arch/x86/kernel/i8259_64.c                 |    1 
 arch/x86/kernel/smp_32.c                   |  165 +++-------
 arch/x86/kernel/smp_64.c                   |  175 ++---------
 arch/x86/kernel/smpboot_32.c               |    4 
 arch/x86/kernel/smpcommon_32.c             |   34 --
 include/asm-powerpc/smp.h                  |    5 
 include/asm-x86/hw_irq_32.h                |    1 
 include/asm-x86/hw_irq_64.h                |    4 
 include/asm-x86/mach-default/entry_arch.h  |    1 
 include/asm-x86/mach-default/irq_vectors.h |    1 
 include/linux/smp.h                        |   30 +
 kernel/Makefile                            |    2 
 kernel/smp.c                               |  316 +++++++++++++++++++++
 20 files changed, 533 insertions(+), 619 deletions(-)

When/if more archs are converted, it'll of course look even better.
Comments? I've verified that this builts and boots on x86, x86-64
and powerpc. IA64 may still need a one-liner change, I don't have
access to that platform anymore so I can't verify that myself. It
does build, however :-)

The patch series is also available in the 'generic-ipi' branch from

git://git.kernel.dk/linux-2.6-block.git

and the 'io-cpu-affinity' branch is directly based on this.

-- 
Jens Axboe

[PATCH 1/5] Add generic helpers for arch IPI function calls

[Jens Axboe]

This adds kernel/smp.c which contains helpers for IPI function calls. In
addition to supporting the existing smp_call_function() in a more efficient
manner, it also adds a more scalable variant called smp_call_function_single()
for calling a given function on a single CPU only.

The core of this is based on the x86-64 patch from Nick Piggin.

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
---
 include/linux/smp.h |   30 +++++-
 kernel/Makefile     |    2 +-
 kernel/smp.c        |  316 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 345 insertions(+), 3 deletions(-)
 create mode 100644 kernel/smp.c

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 55232cc..19b217e 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -7,9 +7,19 @@
  */
 
 #include <linux/errno.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/cpumask.h>
 
 extern void cpu_idle(void);
 
+struct call_single_data {
+	struct list_head list;
+	void (*func) (void *info);
+	void *info;
+	unsigned int flags;
+};
+
 #ifdef CONFIG_SMP
 
 #include <linux/preempt.h>
@@ -50,12 +60,27 @@ extern int __cpu_up(unsigned int cpunum);
 extern void smp_cpus_done(unsigned int max_cpus);
 
 /*
- * Call a function on all other processors
+ * Call a function on all other processors - arch exported functions
  */
 int smp_call_function(void(*func)(void *info), void *info, int retry, int wait);
-
 int smp_call_function_single(int cpuid, void (*func) (void *info), void *info,
 				int retry, int wait);
+void __smp_call_function_single(int cpuid, struct call_single_data *data);
+
+/*
+ * Generic helpers
+ */
+int generic_smp_call_function_single(int cpuid, void (*func) (void *info),
+					void *info, int wait,
+					void (*send_ipi)(int cpu));
+int generic_smp_call_function(void (*func) (void *info), void *info, int wait,
+				cpumask_t mask, void (*send_ipi)(cpumask_t));
+void generic_exec_single(int cpu, struct call_single_data *data,
+			 void (*send_ipi)(int cpu));
+void generic_smp_call_function_single_interrupt(void);
+void generic_smp_call_function_interrupt(void);
+
+extern spinlock_t call_function_lock;
 
 /*
  * Call a function on all processors
@@ -92,6 +117,7 @@ static inline int up_smp_call_function(void (*func)(void *), void *info)
 }
 #define smp_call_function(func, info, retry, wait) \
 			(up_smp_call_function(func, info))
+
 #define on_each_cpu(func,info,retry,wait)	\
 	({					\
 		local_irq_disable();		\
diff --git a/kernel/Makefile b/kernel/Makefile
index 6c584c5..a805976 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -27,7 +27,7 @@ obj-$(CONFIG_RT_MUTEXES) += rtmutex.o
 obj-$(CONFIG_DEBUG_RT_MUTEXES) += rtmutex-debug.o
 obj-$(CONFIG_RT_MUTEX_TESTER) += rtmutex-tester.o
 obj-$(CONFIG_GENERIC_ISA_DMA) += dma.o
-obj-$(CONFIG_SMP) += cpu.o spinlock.o
+obj-$(CONFIG_SMP) += cpu.o spinlock.o smp.o
 obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
 obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
 obj-$(CONFIG_UID16) += uid16.o
diff --git a/kernel/smp.c b/kernel/smp.c
new file mode 100644
index 0000000..ab2da5c
--- /dev/null
+++ b/kernel/smp.c
@@ -0,0 +1,316 @@
+/*
+ * Generic helpers for smp ipi calls
+ *
+ * (C) Jens Axboe <jens.axboe@oracle.com> 2008
+ *
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/rcupdate.h>
+#include <linux/smp.h>
+
+static DEFINE_PER_CPU(struct call_single_queue, call_single_queue);
+static LIST_HEAD(call_function_queue);
+__cacheline_aligned_in_smp DEFINE_SPINLOCK(call_function_lock);
+
+enum {
+	CSD_FLAG_WAIT		= 0x01,
+	CSD_FLAG_ALLOC		= 0x02,
+	CSD_FLAG_FALLBACK	= 0x04,
+};
+
+struct call_function_data {
+	struct call_single_data csd;
+	spinlock_t lock;
+	unsigned int refs;
+	cpumask_t cpumask;
+	struct rcu_head rcu_head;
+};
+
+struct call_single_queue {
+	struct list_head list;
+	spinlock_t lock;
+	int activated;
+};
+
+/*
+ * Fallback data to use, if the dyn allocation fails
+ */
+static struct call_function_data call_data_fallback;
+static unsigned long call_fallback_used;
+
+static int __cpuinit init_call_single_data(void)
+{
+	int i;
+
+	for_each_cpu_mask(i, cpu_possible_map) {
+		struct call_single_queue *q = &per_cpu(call_single_queue, i);
+
+		spin_lock_init(&q->lock);
+		INIT_LIST_HEAD(&q->list);
+		q->activated = 0;
+	}
+	return 0;
+}
+core_initcall(init_call_single_data);
+
+/*
+ * Insert a previously allocated call_single_data element for execution
+ * on the given CPU. data must already have ->func, ->info, and ->flags set.
+ */
+void generic_exec_single(int cpu, struct call_single_data *data,
+			 void (*send_ipi)(int cpu))
+{
+	struct call_single_queue *dst;
+	unsigned long flags;
+	int wait = data->flags & CSD_FLAG_WAIT, ipi = 0;
+
+	INIT_LIST_HEAD(&data->list);
+
+	dst = &per_cpu(call_single_queue, cpu);
+	spin_lock_irqsave(&dst->lock, flags);
+	list_add_tail(&data->list, &dst->list);
+
+	smp_rmb();
+	if (!dst->activated) {
+		dst->activated = 1;
+		ipi = 1;
+	}
+
+	spin_unlock_irqrestore(&dst->lock, flags);
+
+	if (ipi)
+		send_ipi(cpu);
+
+	if (wait) {
+		/* Wait for response */
+		smp_rmb();
+		while (data->flags) {
+			cpu_relax();
+			smp_rmb();
+		}
+	}
+}
+
+/*
+ * Execute func(info) on the given cpu
+ */
+int generic_smp_call_function_single(int cpu, void (*func) (void *info),
+				     void *info, int wait,
+				     void (*send_ipi)(int cpu))
+{
+	unsigned long flags;
+	/* prevent preemption and reschedule on another processor */
+	int me = get_cpu();
+	int ret = 0;
+
+	/* Can deadlock when called with interrupts disabled */
+	WARN_ON(wait && irqs_disabled());
+
+	if (cpu == me) {
+		local_irq_save(flags);
+		func(info);
+		local_irq_restore(flags);
+	} else {
+		struct call_single_data d;
+		struct call_single_data *data;
+
+		if (!wait) {
+			data = kmalloc(sizeof(*data), GFP_ATOMIC);
+			if (unlikely(!data)) {
+				ret = -ENOMEM;
+				goto out;
+			}
+			data->flags = CSD_FLAG_ALLOC;
+		} else {
+			data = &d;
+			data->flags = CSD_FLAG_WAIT;
+		}
+
+		data->func = func;
+		data->info = info;
+		generic_exec_single(cpu, data, send_ipi);
+	}
+out:
+	put_cpu();
+	return ret;
+}
+
+static void rcu_free_call_data(struct rcu_head *head)
+{
+	struct call_function_data *cfd;
+
+	cfd = container_of(head, struct call_function_data, rcu_head);
+	kfree(cfd);
+}
+
+static void call_func_data_free(struct call_function_data *data)
+{
+	if (data->csd.flags & CSD_FLAG_ALLOC)
+		call_rcu(&data->rcu_head, rcu_free_call_data);
+	else
+		clear_bit_unlock(0, &call_fallback_used);
+}
+
+static struct call_function_data *call_func_data_alloc(gfp_t gfp, int wait_done)
+{
+	struct call_function_data *data;
+
+	data = kmalloc(sizeof(*data), gfp);
+	if (likely(data))
+		data->csd.flags = CSD_FLAG_ALLOC;
+	else {
+		while (test_and_set_bit_lock(0, &call_fallback_used))
+			cpu_relax();
+
+		data = &call_data_fallback;
+		data->csd.flags = CSD_FLAG_FALLBACK;
+	}
+
+	if (wait_done)
+		data->csd.flags |= CSD_FLAG_WAIT;
+
+	spin_lock_init(&data->lock);
+	return data;
+}
+
+/*
+ * Execute func(info) on the specified mask of CPUs
+ */
+int generic_smp_call_function(void (*func)(void *info), void *info, int wait,
+				cpumask_t mask,
+				void (*send_ipi)(cpumask_t mask))
+{
+	struct call_function_data *data;
+	cpumask_t allbutself;
+	int num_cpus;
+
+	/* Can deadlock when called with interrupts disabled */
+	WARN_ON(wait && irqs_disabled());
+
+	allbutself = cpu_online_map;
+	cpu_clear(smp_processor_id(), allbutself);
+	cpus_and(mask, mask, allbutself);
+	num_cpus = cpus_weight(mask);
+
+	if (!num_cpus)
+		return 0;
+
+	data = call_func_data_alloc(GFP_ATOMIC, wait);
+	data->csd.func = func;
+	data->csd.info = info;
+	data->refs = num_cpus;
+	data->cpumask = mask;
+
+	local_irq_disable();
+	while (!spin_trylock(&call_function_lock)) {
+		local_irq_enable();
+		cpu_relax();
+		local_irq_disable();
+	}
+
+	list_add_tail_rcu(&data->csd.list, &call_function_queue);
+	spin_unlock(&call_function_lock);
+	local_irq_enable();
+
+	/* Send a message to all CPUs in the map */
+	send_ipi(mask);
+
+	/* optionally wait for the CPUs to complete */
+	if (wait) {
+		smp_rmb();
+		while (data->csd.flags) {
+			cpu_relax();
+			smp_rmb();
+		}
+		call_func_data_free(data);
+	}
+
+	return 0;
+}
+
+/*
+ * Invoked by arch to handle an IPI for call function
+ */
+void generic_smp_call_function_interrupt(void)
+{
+	int cpu = smp_processor_id();
+	struct list_head *pos, *tmp;
+
+	list_for_each_safe_rcu(pos, tmp, &call_function_queue) {
+		struct call_function_data *data;
+		int refs;
+
+		data = list_entry(pos, struct call_function_data, csd.list);
+		if (!cpu_isset(cpu, data->cpumask))
+			continue;
+
+		data->csd.func(data->csd.info);
+		spin_lock(&data->lock);
+		WARN_ON(!cpu_isset(cpu, data->cpumask));
+		cpu_clear(cpu, data->cpumask);
+		WARN_ON(data->refs == 0);
+		data->refs--;
+		refs = data->refs;
+		spin_unlock(&data->lock);
+
+		if (refs)
+			continue;
+
+		WARN_ON(cpus_weight(data->cpumask));
+		spin_lock(&call_function_lock);
+		list_del_rcu(&data->csd.list);
+		spin_unlock(&call_function_lock);
+
+		if (data->csd.flags & CSD_FLAG_WAIT) {
+			smp_wmb();
+			data->csd.flags = 0;
+		} else
+			call_func_data_free(data);
+	}
+}
+
+/*
+ * Invoked by arch to handle an IPI for call function single
+ */
+void generic_smp_call_function_single_interrupt(void)
+{
+	struct call_single_queue *q = &__get_cpu_var(call_single_queue);
+	LIST_HEAD(list);
+
+	/*
+	 * make sure list update is seen
+	 */
+	smp_mb();
+	while (!list_empty(&q->list)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&q->lock, flags);
+		list_replace_init(&q->list, &list);
+		spin_unlock_irqrestore(&q->lock, flags);
+
+		while (!list_empty(&list)) {
+			struct call_single_data *data;
+
+			data = list_entry(list.next, struct call_single_data,
+						list);
+			list_del(&data->list);
+
+			data->func(data->info);
+			if (data->flags & CSD_FLAG_WAIT) {
+				smp_wmb();
+				data->flags = 0;
+			} else if (data->flags & CSD_FLAG_ALLOC)
+				kfree(data);
+		}
+
+		/*
+		 * make sure list update is seen
+		 */
+		smp_mb();
+	}
+
+	smp_wmb();
+	q->activated = 0;
+}
-- 
1.5.4.GIT

[PATCH 2/5] x86: convert to generic helpers for IPI function calls

[Jens Axboe]

This converts x86 to use the new helpers for smp_call_function() and
frieds, and adds support for smp_call_function_single().

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
---
 arch/x86/kernel/smp_32.c                   |  165 +++++++++-------------------
 arch/x86/kernel/smpboot_32.c               |    4 +
 arch/x86/kernel/smpcommon_32.c             |   34 ------
 include/asm-x86/hw_irq_32.h                |    1 +
 include/asm-x86/mach-default/entry_arch.h  |    1 +
 include/asm-x86/mach-default/irq_vectors.h |    1 +
 6 files changed, 61 insertions(+), 145 deletions(-)

diff --git a/arch/x86/kernel/smp_32.c b/arch/x86/kernel/smp_32.c
index dc0cde9..8d12130 100644
--- a/arch/x86/kernel/smp_32.c
+++ b/arch/x86/kernel/smp_32.c
@@ -476,64 +476,29 @@ static void native_smp_send_reschedule(int cpu)
 	send_IPI_mask(cpumask_of_cpu(cpu), RESCHEDULE_VECTOR);
 }
 
-/*
- * Structure and data for smp_call_function(). This is designed to minimise
- * static memory requirements. It also looks cleaner.
- */
-static DEFINE_SPINLOCK(call_lock);
-
-struct call_data_struct {
-	void (*func) (void *info);
-	void *info;
-	atomic_t started;
-	atomic_t finished;
-	int wait;
-};
-
 void lock_ipi_call_lock(void)
 {
-	spin_lock_irq(&call_lock);
+	spin_lock_irq(&call_function_lock);
 }
 
 void unlock_ipi_call_lock(void)
 {
-	spin_unlock_irq(&call_lock);
+	spin_unlock_irq(&call_function_lock);
 }
 
-static struct call_data_struct *call_data;
-
-static void __smp_call_function(void (*func) (void *info), void *info,
-				int nonatomic, int wait)
+static void send_call_function_ipi(cpumask_t mask)
 {
-	struct call_data_struct data;
-	int cpus = num_online_cpus() - 1;
-
-	if (!cpus)
-		return;
-
-	data.func = func;
-	data.info = info;
-	atomic_set(&data.started, 0);
-	data.wait = wait;
-	if (wait)
-		atomic_set(&data.finished, 0);
-
-	call_data = &data;
-	mb();
-	
-	/* Send a message to all other CPUs and wait for them to respond */
-	send_IPI_allbutself(CALL_FUNCTION_VECTOR);
+	cpumask_t allbutself;
 
-	/* Wait for response */
-	while (atomic_read(&data.started) != cpus)
-		cpu_relax();
+	allbutself = cpu_online_map;
+	cpu_clear(smp_processor_id(), allbutself);
 
-	if (wait)
-		while (atomic_read(&data.finished) != cpus)
-			cpu_relax();
+	if (cpus_equal(mask, allbutself))
+		send_IPI_allbutself(CALL_FUNCTION_VECTOR);
+	else
+		send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
 }
 
-
 /**
  * smp_call_function_mask(): Run a function on a set of other CPUs.
  * @mask: The set of cpus to run on.  Must not include the current cpu.
@@ -554,54 +519,41 @@ native_smp_call_function_mask(cpumask_t mask,
 			      void (*func)(void *), void *info,
 			      int wait)
 {
-	struct call_data_struct data;
-	cpumask_t allbutself;
-	int cpus;
-
-	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
-
-	/* Holding any lock stops cpus from going down. */
-	spin_lock(&call_lock);
-
-	allbutself = cpu_online_map;
-	cpu_clear(smp_processor_id(), allbutself);
-
-	cpus_and(mask, mask, allbutself);
-	cpus = cpus_weight(mask);
-
-	if (!cpus) {
-		spin_unlock(&call_lock);
-		return 0;
-	}
-
-	data.func = func;
-	data.info = info;
-	atomic_set(&data.started, 0);
-	data.wait = wait;
-	if (wait)
-		atomic_set(&data.finished, 0);
-
-	call_data = &data;
-	mb();
+	return generic_smp_call_function(func, info, wait, mask,
+						send_call_function_ipi);
+}
 
-	/* Send a message to other CPUs */
-	if (cpus_equal(mask, allbutself))
-		send_IPI_allbutself(CALL_FUNCTION_VECTOR);
-	else
-		send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
+static void send_cfs_ipi(int cpu)
+{
+	cpumask_t mask = cpumask_of_cpu(cpu);
 
-	/* Wait for response */
-	while (atomic_read(&data.started) != cpus)
-		cpu_relax();
+	send_IPI_mask(mask, CALL_FUNCTION_SINGLE_VECTOR);
+}
 
-	if (wait)
-		while (atomic_read(&data.finished) != cpus)
-			cpu_relax();
-	spin_unlock(&call_lock);
+void __smp_call_function_single(int cpu, struct call_single_data *data)
+{
+	generic_exec_single(cpu, data, send_cfs_ipi);
+}
 
-	return 0;
+/*
+ * smp_call_function_single - Run a function on a specific CPU
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @nonatomic: Currently unused.
+ * @wait: If true, wait until function has completed on other CPUs.
+ *
+ * Retrurns 0 on success, else a negative status code.
+ *
+ * Does not return until the remote CPU is nearly ready to execute <func>
+ * or is or has executed.
+ */
+int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
+			      int nonatomic, int wait)
+{
+	return generic_smp_call_function_single(cpu, func, info, wait,
+						send_cfs_ipi);
 }
+EXPORT_SYMBOL(smp_call_function_single);
 
 static void stop_this_cpu (void * dummy)
 {
@@ -622,14 +574,10 @@ static void stop_this_cpu (void * dummy)
 
 static void native_smp_send_stop(void)
 {
-	/* Don't deadlock on the call lock in panic */
-	int nolock = !spin_trylock(&call_lock);
 	unsigned long flags;
 
 	local_irq_save(flags);
-	__smp_call_function(stop_this_cpu, NULL, 0, 0);
-	if (!nolock)
-		spin_unlock(&call_lock);
+	smp_call_function(stop_this_cpu, NULL, 0, 0);
 	disable_local_APIC();
 	local_irq_restore(flags);
 }
@@ -647,29 +595,24 @@ void smp_reschedule_interrupt(struct pt_regs *regs)
 
 void smp_call_function_interrupt(struct pt_regs *regs)
 {
-	void (*func) (void *info) = call_data->func;
-	void *info = call_data->info;
-	int wait = call_data->wait;
-
 	ack_APIC_irq();
-	/*
-	 * Notify initiating CPU that I've grabbed the data and am
-	 * about to execute the function
-	 */
-	mb();
-	atomic_inc(&call_data->started);
-	/*
-	 * At this point the info structure may be out of scope unless wait==1
-	 */
 	irq_enter();
-	(*func)(info);
+
+	generic_smp_call_function_interrupt();
+
 	__get_cpu_var(irq_stat).irq_call_count++;
 	irq_exit();
+}
 
-	if (wait) {
-		mb();
-		atomic_inc(&call_data->finished);
-	}
+void smp_call_function_single_interrupt(void)
+{
+	ack_APIC_irq();
+	irq_enter();
+
+	generic_smp_call_function_single_interrupt();
+
+	__get_cpu_var(irq_stat).irq_call_count++;
+	irq_exit();
 }
 
 static int convert_apicid_to_cpu(int apic_id)
diff --git a/arch/x86/kernel/smpboot_32.c b/arch/x86/kernel/smpboot_32.c
index 579b9b7..d250388 100644
--- a/arch/x86/kernel/smpboot_32.c
+++ b/arch/x86/kernel/smpboot_32.c
@@ -1304,6 +1304,10 @@ void __init smp_intr_init(void)
 
 	/* IPI for generic function call */
 	set_intr_gate(CALL_FUNCTION_VECTOR, call_function_interrupt);
+
+	/* IPI for single call function */
+	set_intr_gate(CALL_FUNCTION_SINGLE_VECTOR,
+					call_function_single_interrupt);
 }
 
 /*
diff --git a/arch/x86/kernel/smpcommon_32.c b/arch/x86/kernel/smpcommon_32.c
index 8bc38af..4590a67 100644
--- a/arch/x86/kernel/smpcommon_32.c
+++ b/arch/x86/kernel/smpcommon_32.c
@@ -46,37 +46,3 @@ int smp_call_function(void (*func) (void *info), void *info, int nonatomic,
 	return smp_call_function_mask(cpu_online_map, func, info, wait);
 }
 EXPORT_SYMBOL(smp_call_function);
-
-/**
- * smp_call_function_single - Run a function on a specific CPU
- * @cpu: The target CPU.  Cannot be the calling CPU.
- * @func: The function to run. This must be fast and non-blocking.
- * @info: An arbitrary pointer to pass to the function.
- * @nonatomic: Unused.
- * @wait: If true, wait until function has completed on other CPUs.
- *
- * Returns 0 on success, else a negative status code.
- *
- * If @wait is true, then returns once @func has returned; otherwise
- * it returns just before the target cpu calls @func.
- */
-int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
-			     int nonatomic, int wait)
-{
-	/* prevent preemption and reschedule on another processor */
-	int ret;
-	int me = get_cpu();
-	if (cpu == me) {
-		local_irq_disable();
-		func(info);
-		local_irq_enable();
-		put_cpu();
-		return 0;
-	}
-
-	ret = smp_call_function_mask(cpumask_of_cpu(cpu), func, info, wait);
-
-	put_cpu();
-	return ret;
-}
-EXPORT_SYMBOL(smp_call_function_single);
diff --git a/include/asm-x86/hw_irq_32.h b/include/asm-x86/hw_irq_32.h
index ea88054..a87b132 100644
--- a/include/asm-x86/hw_irq_32.h
+++ b/include/asm-x86/hw_irq_32.h
@@ -32,6 +32,7 @@ extern void (*const interrupt[NR_IRQS])(void);
 void reschedule_interrupt(void);
 void invalidate_interrupt(void);
 void call_function_interrupt(void);
+void call_function_single_interrupt(void);
 #endif
 
 #ifdef CONFIG_X86_LOCAL_APIC
diff --git a/include/asm-x86/mach-default/entry_arch.h b/include/asm-x86/mach-default/entry_arch.h
index bc86146..9283b60 100644
--- a/include/asm-x86/mach-default/entry_arch.h
+++ b/include/asm-x86/mach-default/entry_arch.h
@@ -13,6 +13,7 @@
 BUILD_INTERRUPT(reschedule_interrupt,RESCHEDULE_VECTOR)
 BUILD_INTERRUPT(invalidate_interrupt,INVALIDATE_TLB_VECTOR)
 BUILD_INTERRUPT(call_function_interrupt,CALL_FUNCTION_VECTOR)
+BUILD_INTERRUPT(call_function_single_interrupt,CALL_FUNCTION_SINGLE_VECTOR)
 #endif
 
 /*
diff --git a/include/asm-x86/mach-default/irq_vectors.h b/include/asm-x86/mach-default/irq_vectors.h
index 881c63c..ed7d495 100644
--- a/include/asm-x86/mach-default/irq_vectors.h
+++ b/include/asm-x86/mach-default/irq_vectors.h
@@ -48,6 +48,7 @@
 #define INVALIDATE_TLB_VECTOR	0xfd
 #define RESCHEDULE_VECTOR	0xfc
 #define CALL_FUNCTION_VECTOR	0xfb
+#define CALL_FUNCTION_SINGLE_VECTOR	0xfa
 
 #define THERMAL_APIC_VECTOR	0xf0
 /*
-- 
1.5.4.GIT

[PATCH 3/5] x86-64: convert to generic helpers for IPI function calls

[Jens Axboe]

This converts x86-64 to use the new helpers for smp_call_function() and
frieds, and adds support for smp_call_function_single().

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
---
 arch/x86/kernel/entry_64.S  |    3 +
 arch/x86/kernel/i8259_64.c  |    1 +
 arch/x86/kernel/smp_64.c    |  175 ++++++++++---------------------------------
 include/asm-x86/hw_irq_64.h |    4 +-
 4 files changed, 46 insertions(+), 137 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index c20c9e7..22caf56 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -713,6 +713,9 @@ END(invalidate_interrupt\num)
 ENTRY(call_function_interrupt)
 	apicinterrupt CALL_FUNCTION_VECTOR,smp_call_function_interrupt
 END(call_function_interrupt)
+ENTRY(call_function_single_interrupt)
+	apicinterrupt CALL_FUNCTION_SINGLE_VECTOR,smp_call_function_single_interrupt
+END(call_function_single_interrupt)
 ENTRY(irq_move_cleanup_interrupt)
 	apicinterrupt IRQ_MOVE_CLEANUP_VECTOR,smp_irq_move_cleanup_interrupt
 END(irq_move_cleanup_interrupt)
diff --git a/arch/x86/kernel/i8259_64.c b/arch/x86/kernel/i8259_64.c
index fa57a15..2b0b6d2 100644
--- a/arch/x86/kernel/i8259_64.c
+++ b/arch/x86/kernel/i8259_64.c
@@ -493,6 +493,7 @@ void __init native_init_IRQ(void)
 
 	/* IPI for generic function call */
 	set_intr_gate(CALL_FUNCTION_VECTOR, call_function_interrupt);
+	set_intr_gate(CALL_FUNCTION_SINGLE_VECTOR, call_function_single_interrupt);
 
 	/* Low priority IPI to cleanup after moving an irq */
 	set_intr_gate(IRQ_MOVE_CLEANUP_VECTOR, irq_move_cleanup_interrupt);
diff --git a/arch/x86/kernel/smp_64.c b/arch/x86/kernel/smp_64.c
index 2fd74b0..6002274 100644
--- a/arch/x86/kernel/smp_64.c
+++ b/arch/x86/kernel/smp_64.c
@@ -295,111 +295,27 @@ void smp_send_reschedule(int cpu)
 	send_IPI_mask(cpumask_of_cpu(cpu), RESCHEDULE_VECTOR);
 }
 
-/*
- * Structure and data for smp_call_function(). This is designed to minimise
- * static memory requirements. It also looks cleaner.
- */
-static DEFINE_SPINLOCK(call_lock);
-
-struct call_data_struct {
-	void (*func) (void *info);
-	void *info;
-	atomic_t started;
-	atomic_t finished;
-	int wait;
-};
-
-static struct call_data_struct * call_data;
-
 void lock_ipi_call_lock(void)
 {
-	spin_lock_irq(&call_lock);
+	spin_lock_irq(&call_function_lock);
 }
 
 void unlock_ipi_call_lock(void)
 {
-	spin_unlock_irq(&call_lock);
+	spin_unlock_irq(&call_function_lock);
 }
 
-/*
- * this function sends a 'generic call function' IPI to all other CPU
- * of the system defined in the mask.
- */
-static int __smp_call_function_mask(cpumask_t mask,
-				    void (*func)(void *), void *info,
-				    int wait)
+static void send_cfs_ipi(int cpu)
 {
-	struct call_data_struct data;
-	cpumask_t allbutself;
-	int cpus;
-
-	allbutself = cpu_online_map;
-	cpu_clear(smp_processor_id(), allbutself);
-
-	cpus_and(mask, mask, allbutself);
-	cpus = cpus_weight(mask);
-
-	if (!cpus)
-		return 0;
-
-	data.func = func;
-	data.info = info;
-	atomic_set(&data.started, 0);
-	data.wait = wait;
-	if (wait)
-		atomic_set(&data.finished, 0);
-
-	call_data = &data;
-	wmb();
-
-	/* Send a message to other CPUs */
-	if (cpus_equal(mask, allbutself))
-		send_IPI_allbutself(CALL_FUNCTION_VECTOR);
-	else
-		send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
-
-	/* Wait for response */
-	while (atomic_read(&data.started) != cpus)
-		cpu_relax();
+	cpumask_t mask = cpumask_of_cpu(cpu);
 
-	if (!wait)
-		return 0;
-
-	while (atomic_read(&data.finished) != cpus)
-		cpu_relax();
-
-	return 0;
+	send_IPI_mask(mask, CALL_FUNCTION_SINGLE_VECTOR);
 }
-/**
- * smp_call_function_mask(): Run a function on a set of other CPUs.
- * @mask: The set of cpus to run on.  Must not include the current cpu.
- * @func: The function to run. This must be fast and non-blocking.
- * @info: An arbitrary pointer to pass to the function.
- * @wait: If true, wait (atomically) until function has completed on other CPUs.
- *
- * Returns 0 on success, else a negative status code.
- *
- * If @wait is true, then returns once @func has returned; otherwise
- * it returns just before the target cpu calls @func.
- *
- * You must not call this function with disabled interrupts or from a
- * hardware interrupt handler or from a bottom half handler.
- */
-int smp_call_function_mask(cpumask_t mask,
-			   void (*func)(void *), void *info,
-			   int wait)
-{
-	int ret;
-
-	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
 
-	spin_lock(&call_lock);
-	ret = __smp_call_function_mask(mask, func, info, wait);
-	spin_unlock(&call_lock);
-	return ret;
+void __smp_call_function_single(int cpu, struct call_single_data *data)
+{
+	generic_exec_single(cpu, data, send_cfs_ipi);
 }
-EXPORT_SYMBOL(smp_call_function_mask);
 
 /*
  * smp_call_function_single - Run a function on a specific CPU
@@ -413,30 +329,26 @@ EXPORT_SYMBOL(smp_call_function_mask);
  * Does not return until the remote CPU is nearly ready to execute <func>
  * or is or has executed.
  */
-
-int smp_call_function_single (int cpu, void (*func) (void *info), void *info,
+int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
 			      int nonatomic, int wait)
 {
-	/* prevent preemption and reschedule on another processor */
-	int ret, me = get_cpu();
-
-	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
-
-	if (cpu == me) {
-		local_irq_disable();
-		func(info);
-		local_irq_enable();
-		put_cpu();
-		return 0;
-	}
+	return generic_smp_call_function_single(cpu, func, info, wait,
+						send_cfs_ipi);
+}
+EXPORT_SYMBOL(smp_call_function_single);
 
-	ret = smp_call_function_mask(cpumask_of_cpu(cpu), func, info, wait);
+static void send_call_function_ipi(cpumask_t mask)
+{
+	cpumask_t allbutself;
 
-	put_cpu();
-	return ret;
+	allbutself = cpu_online_map;
+	cpu_clear(smp_processor_id(), allbutself);
+
+	if (cpus_equal(mask, allbutself))
+		send_IPI_allbutself(CALL_FUNCTION_VECTOR);
+	else
+		send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
 }
-EXPORT_SYMBOL(smp_call_function_single);
 
 /*
  * smp_call_function - run a function on all other CPUs.
@@ -456,7 +368,8 @@ EXPORT_SYMBOL(smp_call_function_single);
 int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
 			int wait)
 {
-	return smp_call_function_mask(cpu_online_map, func, info, wait);
+	return generic_smp_call_function(func, info, wait, cpu_online_map,
+						send_call_function_ipi);
 }
 EXPORT_SYMBOL(smp_call_function);
 
@@ -474,18 +387,13 @@ static void stop_this_cpu(void *dummy)
 
 void smp_send_stop(void)
 {
-	int nolock;
 	unsigned long flags;
 
 	if (reboot_force)
 		return;
 
-	/* Don't deadlock on the call lock in panic */
-	nolock = !spin_trylock(&call_lock);
 	local_irq_save(flags);
-	__smp_call_function_mask(cpu_online_map, stop_this_cpu, NULL, 0);
-	if (!nolock)
-		spin_unlock(&call_lock);
+	smp_call_function(stop_this_cpu, NULL, 0, 0);
 	disable_local_APIC();
 	local_irq_restore(flags);
 }
@@ -503,28 +411,25 @@ asmlinkage void smp_reschedule_interrupt(void)
 
 asmlinkage void smp_call_function_interrupt(void)
 {
-	void (*func) (void *info) = call_data->func;
-	void *info = call_data->info;
-	int wait = call_data->wait;
+	ack_APIC_irq();
+	exit_idle();
+	irq_enter();
+
+	generic_smp_call_function_interrupt();
+
+	add_pda(irq_call_count, 1);
+	irq_exit();
+}
 
+asmlinkage void smp_call_function_single_interrupt(void)
+{
 	ack_APIC_irq();
-	/*
-	 * Notify initiating CPU that I've grabbed the data and am
-	 * about to execute the function
-	 */
-	mb();
-	atomic_inc(&call_data->started);
-	/*
-	 * At this point the info structure may be out of scope unless wait==1
-	 */
 	exit_idle();
 	irq_enter();
-	(*func)(info);
+
+	generic_smp_call_function_single_interrupt();
+
 	add_pda(irq_call_count, 1);
 	irq_exit();
-	if (wait) {
-		mb();
-		atomic_inc(&call_data->finished);
-	}
 }
 
diff --git a/include/asm-x86/hw_irq_64.h b/include/asm-x86/hw_irq_64.h
index 312a58d..06ac80c 100644
--- a/include/asm-x86/hw_irq_64.h
+++ b/include/asm-x86/hw_irq_64.h
@@ -68,8 +68,7 @@
 #define ERROR_APIC_VECTOR	0xfe
 #define RESCHEDULE_VECTOR	0xfd
 #define CALL_FUNCTION_VECTOR	0xfc
-/* fb free - please don't readd KDB here because it's useless
-   (hint - think what a NMI bit does to a vector) */
+#define CALL_FUNCTION_SINGLE_VECTOR	0xfb
 #define THERMAL_APIC_VECTOR	0xfa
 #define THRESHOLD_APIC_VECTOR   0xf9
 /* f8 free */
@@ -102,6 +101,7 @@ void spurious_interrupt(void);
 void error_interrupt(void);
 void reschedule_interrupt(void);
 void call_function_interrupt(void);
+void call_function_single_interrupt(void);
 void irq_move_cleanup_interrupt(void);
 void invalidate_interrupt0(void);
 void invalidate_interrupt1(void);
-- 
1.5.4.GIT

[PATCH 4/5] powerpc: convert to generic helpers for IPI function calls

[Jens Axboe]

This converts ppc to use the new helpers for smp_call_function() and
frieds, and adds support for smp_call_function_single().

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
---
 arch/powerpc/kernel/smp.c               |  223 ++++---------------------------
 arch/powerpc/platforms/cell/interrupt.c |    1 +
 arch/powerpc/platforms/ps3/smp.c        |    7 +-
 arch/powerpc/platforms/pseries/xics.c   |    6 +-
 arch/powerpc/sysdev/mpic.c              |    2 +-
 include/asm-powerpc/smp.h               |    5 +-
 6 files changed, 36 insertions(+), 208 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index be35ffa..c94c80c 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -72,12 +72,8 @@ struct smp_ops_t *smp_ops;
 
 static volatile unsigned int cpu_callin_map[NR_CPUS];
 
-void smp_call_function_interrupt(void);
-
 int smt_enabled_at_boot = 1;
 
-static int ipi_fail_ok;
-
 static void (*crash_ipi_function_ptr)(struct pt_regs *) = NULL;
 
 #ifdef CONFIG_PPC64
@@ -99,12 +95,15 @@ void smp_message_recv(int msg)
 {
 	switch(msg) {
 	case PPC_MSG_CALL_FUNCTION:
-		smp_call_function_interrupt();
+		generic_smp_call_function_interrupt();
 		break;
 	case PPC_MSG_RESCHEDULE:
 		/* XXX Do we have to do this? */
 		set_need_resched();
 		break;
+	case PPC_MSG_CALL_FUNC_SINGLE:
+		generic_smp_call_function_single_interrupt();
+		break;
 	case PPC_MSG_DEBUGGER_BREAK:
 		if (crash_ipi_function_ptr) {
 			crash_ipi_function_ptr(get_irq_regs());
@@ -154,215 +153,47 @@ static void stop_this_cpu(void *dummy)
 		;
 }
 
-/*
- * Structure and data for smp_call_function(). This is designed to minimise
- * static memory requirements. It also looks cleaner.
- * Stolen from the i386 version.
- */
-static  __cacheline_aligned_in_smp DEFINE_SPINLOCK(call_lock);
-
-static struct call_data_struct {
-	void (*func) (void *info);
-	void *info;
-	atomic_t started;
-	atomic_t finished;
-	int wait;
-} *call_data;
-
-/* delay of at least 8 seconds */
-#define SMP_CALL_TIMEOUT	8
-
-/*
- * These functions send a 'generic call function' IPI to other online
- * CPUS in the system.
- *
- * [SUMMARY] Run a function on other CPUs.
- * <func> The function to run. This must be fast and non-blocking.
- * <info> An arbitrary pointer to pass to the function.
- * <nonatomic> currently unused.
- * <wait> If true, wait (atomically) until function has completed on other CPUs.
- * [RETURNS] 0 on success, else a negative status code. Does not return until
- * remote CPUs are nearly ready to execute <<func>> or are or have executed.
- * <map> is a cpu map of the cpus to send IPI to.
- *
- * You must not call this function with disabled interrupts or from a
- * hardware interrupt handler or from a bottom half handler.
- */
-static int __smp_call_function_map(void (*func) (void *info), void *info,
-				   int nonatomic, int wait, cpumask_t map)
+static void send_call_function_ipi(cpumask_t mask)
 {
-	struct call_data_struct data;
-	int ret = -1, num_cpus;
-	int cpu;
-	u64 timeout;
-
-	if (unlikely(smp_ops == NULL))
-		return ret;
-
-	data.func = func;
-	data.info = info;
-	atomic_set(&data.started, 0);
-	data.wait = wait;
-	if (wait)
-		atomic_set(&data.finished, 0);
-
-	/* remove 'self' from the map */
-	if (cpu_isset(smp_processor_id(), map))
-		cpu_clear(smp_processor_id(), map);
-
-	/* sanity check the map, remove any non-online processors. */
-	cpus_and(map, map, cpu_online_map);
-
-	num_cpus = cpus_weight(map);
-	if (!num_cpus)
-		goto done;
+	unsigned int cpu;
 
-	call_data = &data;
-	smp_wmb();
-	/* Send a message to all CPUs in the map */
-	for_each_cpu_mask(cpu, map)
+	for_each_cpu_mask(cpu, mask)
 		smp_ops->message_pass(cpu, PPC_MSG_CALL_FUNCTION);
-
-	timeout = get_tb() + (u64) SMP_CALL_TIMEOUT * tb_ticks_per_sec;
-
-	/* Wait for indication that they have received the message */
-	while (atomic_read(&data.started) != num_cpus) {
-		HMT_low();
-		if (get_tb() >= timeout) {
-			printk("smp_call_function on cpu %d: other cpus not "
-				"responding (%d)\n", smp_processor_id(),
-				atomic_read(&data.started));
-			if (!ipi_fail_ok)
-				debugger(NULL);
-			goto out;
-		}
-	}
-
-	/* optionally wait for the CPUs to complete */
-	if (wait) {
-		while (atomic_read(&data.finished) != num_cpus) {
-			HMT_low();
-			if (get_tb() >= timeout) {
-				printk("smp_call_function on cpu %d: other "
-					"cpus not finishing (%d/%d)\n",
-					smp_processor_id(),
-					atomic_read(&data.finished),
-					atomic_read(&data.started));
-				debugger(NULL);
-				goto out;
-			}
-		}
-	}
-
- done:
-	ret = 0;
-
- out:
-	call_data = NULL;
-	HMT_medium();
-	return ret;
-}
-
-static int __smp_call_function(void (*func)(void *info), void *info,
-			       int nonatomic, int wait)
-{
-	int ret;
-	spin_lock(&call_lock);
-	ret =__smp_call_function_map(func, info, nonatomic, wait,
-				       cpu_online_map);
-	spin_unlock(&call_lock);
-	return ret;
 }
 
 int smp_call_function(void (*func) (void *info), void *info, int nonatomic,
 			int wait)
 {
-	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	if (unlikely(smp_ops == NULL))
+		return -1;
 
-	return __smp_call_function(func, info, nonatomic, wait);
+	return generic_smp_call_function(func, info, wait, cpu_online_map,
+						send_call_function_ipi);
 }
 EXPORT_SYMBOL(smp_call_function);
 
-int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
-			     int nonatomic, int wait)
+static void send_cfs_ipi(int cpu)
 {
-	cpumask_t map = CPU_MASK_NONE;
-	int ret = 0;
-
-	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
-
-	if (!cpu_online(cpu))
-		return -EINVAL;
-
-	cpu_set(cpu, map);
-	if (cpu != get_cpu()) {
-		spin_lock(&call_lock);
-		ret = __smp_call_function_map(func, info, nonatomic, wait, map);
-		spin_unlock(&call_lock);
-	} else {
-		local_irq_disable();
-		func(info);
-		local_irq_enable();
-	}
-	put_cpu();
-	return ret;
+	smp_ops->message_pass(cpu, PPC_MSG_CALL_FUNC_SINGLE);
 }
-EXPORT_SYMBOL(smp_call_function_single);
 
-void smp_send_stop(void)
+void __smp_call_function_single(int cpu, struct call_single_data *data)
 {
-	int nolock;
-
-	/* It's OK to fail sending the IPI, since the alternative is to
-	 * be stuck forever waiting on the other CPU to take the interrupt.
-	 *
-	 * It's better to at least continue and go through reboot, since this
-	 * function is usually called at panic or reboot time in the first
-	 * place.
-	 */
-	ipi_fail_ok = 1;
-
-	/* Don't deadlock in case we got called through panic */
-	nolock = !spin_trylock(&call_lock);
-	__smp_call_function_map(stop_this_cpu, NULL, 1, 0, cpu_online_map);
-	if (!nolock)
-		spin_unlock(&call_lock);
+	generic_exec_single(cpu, data, send_cfs_ipi);
 }
 
-void smp_call_function_interrupt(void)
+int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
+			      int nonatomic, int wait)
 {
-	void (*func) (void *info);
-	void *info;
-	int wait;
-
-	/* call_data will be NULL if the sender timed out while
-	 * waiting on us to receive the call.
-	 */
-	if (!call_data)
-		return;
-
-	func = call_data->func;
-	info = call_data->info;
-	wait = call_data->wait;
-
-	if (!wait)
-		smp_mb__before_atomic_inc();
+	return generic_smp_call_function_single(cpu, func, info, wait,
+						send_cfs_ipi);
+}
+EXPORT_SYMBOL(smp_call_function_single);
 
-	/*
-	 * Notify initiating CPU that I've grabbed the data and am
-	 * about to execute the function
-	 */
-	atomic_inc(&call_data->started);
-	/*
-	 * At this point the info structure may be out of scope unless wait==1
-	 */
-	(*func)(info);
-	if (wait) {
-		smp_mb__before_atomic_inc();
-		atomic_inc(&call_data->finished);
-	}
+void smp_send_stop(void)
+{
+	generic_smp_call_function(stop_this_cpu, NULL, 0, cpu_online_map,
+					send_call_function_ipi);
 }
 
 extern struct gettimeofday_struct do_gtod;
@@ -594,9 +425,9 @@ int __devinit start_secondary(void *unused)
 
 	secondary_cpu_time_init();
 
-	spin_lock(&call_lock);
+	spin_lock(&call_function_lock);
 	cpu_set(cpu, cpu_online_map);
-	spin_unlock(&call_lock);
+	spin_unlock(&call_function_lock);
 
 	local_irq_enable();
 
diff --git a/arch/powerpc/platforms/cell/interrupt.c b/arch/powerpc/platforms/cell/interrupt.c
index 04f74f9..bc91ec5 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -217,6 +217,7 @@ void iic_request_IPIs(void)
 {
 	iic_request_ipi(PPC_MSG_CALL_FUNCTION, "IPI-call");
 	iic_request_ipi(PPC_MSG_RESCHEDULE, "IPI-resched");
+	iic_request_ipi(PPC_MSG_CALL_FUNC_SINGLE, "IPI-call-single");
 #ifdef CONFIG_DEBUGGER
 	iic_request_ipi(PPC_MSG_DEBUGGER_BREAK, "IPI-debug");
 #endif /* CONFIG_DEBUGGER */
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index f0b12f2..a0927a3 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -105,9 +105,10 @@ static void __init ps3_smp_setup_cpu(int cpu)
 	 * to index needs to be setup.
 	 */
 
-	BUILD_BUG_ON(PPC_MSG_CALL_FUNCTION  != 0);
-	BUILD_BUG_ON(PPC_MSG_RESCHEDULE     != 1);
-	BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK != 3);
+	BUILD_BUG_ON(PPC_MSG_CALL_FUNCTION    != 0);
+	BUILD_BUG_ON(PPC_MSG_RESCHEDULE       != 1);
+	BUILD_BUG_ON(PPC_MSG_CALL_FUNC_SINGLE != 2);
+	BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK   != 3);
 
 	for (i = 0; i < MSG_COUNT; i++) {
 		result = ps3_event_receive_port_setup(cpu, &virqs[i]);
diff --git a/arch/powerpc/platforms/pseries/xics.c b/arch/powerpc/platforms/pseries/xics.c
index ca52b58..31fc69a 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -384,13 +384,11 @@ static irqreturn_t xics_ipi_dispatch(int cpu)
 			mb();
 			smp_message_recv(PPC_MSG_RESCHEDULE);
 		}
-#if 0
-		if (test_and_clear_bit(PPC_MSG_MIGRATE_TASK,
+		if (test_and_clear_bit(PPC_MSG_CALL_FUNC_SINGLE,
 				       &xics_ipi_message[cpu].value)) {
 			mb();
-			smp_message_recv(PPC_MSG_MIGRATE_TASK);
+			smp_message_recv(PPC_MSG_CALL_FUNC_SINGLE);
 		}
-#endif
 #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC)
 		if (test_and_clear_bit(PPC_MSG_DEBUGGER_BREAK,
 				       &xics_ipi_message[cpu].value)) {
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 6ffdda2..4c51691 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1505,7 +1505,7 @@ void mpic_request_ipis(void)
 	static char *ipi_names[] = {
 		"IPI0 (call function)",
 		"IPI1 (reschedule)",
-		"IPI2 (unused)",
+		"IPI2 (call function single)",
 		"IPI3 (debugger break)",
 	};
 	BUG_ON(mpic == NULL);
diff --git a/include/asm-powerpc/smp.h b/include/asm-powerpc/smp.h
index 505f35b..78382f6 100644
--- a/include/asm-powerpc/smp.h
+++ b/include/asm-powerpc/smp.h
@@ -67,10 +67,7 @@ DECLARE_PER_CPU(cpumask_t, cpu_sibling_map);
  * in /proc/interrupts will be wrong!!! --Troy */
 #define PPC_MSG_CALL_FUNCTION   0
 #define PPC_MSG_RESCHEDULE      1
-/* This is unused now */
-#if 0
-#define PPC_MSG_MIGRATE_TASK    2
-#endif
+#define PPC_MSG_CALL_FUNC_SINGLE	2
 #define PPC_MSG_DEBUGGER_BREAK  3
 
 void smp_init_iSeries(void);
-- 
1.5.4.GIT

[PATCH 5/5] ia64: convert to generic helpers for IPI function calls

[Jens Axboe]

This converts ia64 to use the new helpers for smp_call_function() and
frieds, and adds support for smp_call_function_single().

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
---
 arch/ia64/kernel/smp.c |  171 +++++++++++++-----------------------------------
 1 files changed, 45 insertions(+), 126 deletions(-)

diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c
index 4e446aa..fa26528 100644
--- a/arch/ia64/kernel/smp.c
+++ b/arch/ia64/kernel/smp.c
@@ -61,24 +61,9 @@ static struct local_tlb_flush_counts {
 static DEFINE_PER_CPU(unsigned int, shadow_flush_counts[NR_CPUS]) ____cacheline_aligned;
 
 
-/*
- * Structure and data for smp_call_function(). This is designed to minimise static memory
- * requirements. It also looks cleaner.
- */
-static  __cacheline_aligned DEFINE_SPINLOCK(call_lock);
-
-struct call_data_struct {
-	void (*func) (void *info);
-	void *info;
-	long wait;
-	atomic_t started;
-	atomic_t finished;
-};
-
-static volatile struct call_data_struct *call_data;
-
 #define IPI_CALL_FUNC		0
 #define IPI_CPU_STOP		1
+#define IPI_CALL_FUNC_SINGLE	2
 #define IPI_KDUMP_CPU_STOP	3
 
 /* This needs to be cacheline aligned because it is written to by *other* CPUs.  */
@@ -89,13 +74,13 @@ extern void cpu_halt (void);
 void
 lock_ipi_calllock(void)
 {
-	spin_lock_irq(&call_lock);
+	spin_lock_irq(&call_function_lock);
 }
 
 void
 unlock_ipi_calllock(void)
 {
-	spin_unlock_irq(&call_lock);
+	spin_unlock_irq(&call_function_lock);
 }
 
 static void
@@ -139,32 +124,11 @@ handle_IPI (int irq, void *dev_id)
 
 			switch (which) {
 			      case IPI_CALL_FUNC:
-			      {
-				      struct call_data_struct *data;
-				      void (*func)(void *info);
-				      void *info;
-				      int wait;
-
-				      /* release the 'pointer lock' */
-				      data = (struct call_data_struct *) call_data;
-				      func = data->func;
-				      info = data->info;
-				      wait = data->wait;
-
-				      mb();
-				      atomic_inc(&data->started);
-				      /*
-				       * At this point the structure may be gone unless
-				       * wait is true.
-				       */
-				      (*func)(info);
-
-				      /* Notify the sending CPU that the task is done.  */
-				      mb();
-				      if (wait)
-					      atomic_inc(&data->finished);
-			      }
-			      break;
+				generic_smp_call_function_single_interrupt();
+				break;
+			      case IPI_CALL_FUNC_SINGLE:
+				generic_smp_call_function_single_interrupt();
+				break;
 
 			      case IPI_CPU_STOP:
 				stop_this_cpu();
@@ -209,6 +173,15 @@ send_IPI_allbutself (int op)
 	}
 }
 
+static inline void
+send_IPI_mask(cpumask_t mask, int op)
+{
+	unsigned int cpu;
+
+	for_each_cpu_mask(cpu, mask)
+		send_IPI_single(cpu, op);
+}
+
 /*
  * Called with preemption disabled.
  */
@@ -345,62 +318,41 @@ smp_flush_tlb_mm (struct mm_struct *mm)
 	on_each_cpu((void (*)(void *))local_finish_flush_tlb_mm, mm, 1, 1);
 }
 
+static void send_cfs_ipi(int cpu)
+{
+	send_IPI_single(cpu, IPI_CALL_FUNC_SINGLE);
+}
+
+void __smp_call_function_single(int cpu, struct call_single_data *data)
+{
+	generic_exec_single(cpu, data, send_cfs_ipi);
+}
+
 /*
- * Run a function on a specific CPU
- *  <func>	The function to run. This must be fast and non-blocking.
- *  <info>	An arbitrary pointer to pass to the function.
- *  <nonatomic>	Currently unused.
- *  <wait>	If true, wait until function has completed on other CPUs.
- *  [RETURNS]   0 on success, else a negative status code.
+ * smp_call_function_single - Run a function on a specific CPU
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @nonatomic: Currently unused.
+ * @wait: If true, wait until function has completed on other CPUs.
+ *
+ * Retrurns 0 on success, else a negative status code.
  *
  * Does not return until the remote CPU is nearly ready to execute <func>
  * or is or has executed.
  */
-
-int
-smp_call_function_single (int cpuid, void (*func) (void *info), void *info, int nonatomic,
-			  int wait)
+int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
+			      int nonatomic, int wait)
 {
-	struct call_data_struct data;
-	int cpus = 1;
-	int me = get_cpu(); /* prevent preemption and reschedule on another processor */
-
-	if (cpuid == me) {
-		local_irq_disable();
-		func(info);
-		local_irq_enable();
-		put_cpu();
-		return 0;
-	}
-
-	data.func = func;
-	data.info = info;
-	atomic_set(&data.started, 0);
-	data.wait = wait;
-	if (wait)
-		atomic_set(&data.finished, 0);
-
-	spin_lock_bh(&call_lock);
-
-	call_data = &data;
-	mb();	/* ensure store to call_data precedes setting of IPI_CALL_FUNC */
-  	send_IPI_single(cpuid, IPI_CALL_FUNC);
-
-	/* Wait for response */
-	while (atomic_read(&data.started) != cpus)
-		cpu_relax();
-
-	if (wait)
-		while (atomic_read(&data.finished) != cpus)
-			cpu_relax();
-	call_data = NULL;
-
-	spin_unlock_bh(&call_lock);
-	put_cpu();
-	return 0;
+	return generic_smp_call_function_single(cpu, func, info, wait,
+						send_cfs_ipi);
 }
 EXPORT_SYMBOL(smp_call_function_single);
 
+static void send_call_function_ipi(cpumask_t mask)
+{
+	send_IPI_mask(mask, IPI_CALL_FUNC);
+}
+
 /*
  * this function sends a 'generic call function' IPI to all other CPUs
  * in the system.
@@ -423,41 +375,8 @@ EXPORT_SYMBOL(smp_call_function_single);
 int
 smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wait)
 {
-	struct call_data_struct data;
-	int cpus;
-
-	spin_lock(&call_lock);
-	cpus = num_online_cpus() - 1;
-	if (!cpus) {
-		spin_unlock(&call_lock);
-		return 0;
-	}
-
-	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
-
-	data.func = func;
-	data.info = info;
-	atomic_set(&data.started, 0);
-	data.wait = wait;
-	if (wait)
-		atomic_set(&data.finished, 0);
-
-	call_data = &data;
-	mb();	/* ensure store to call_data precedes setting of IPI_CALL_FUNC */
-	send_IPI_allbutself(IPI_CALL_FUNC);
-
-	/* Wait for response */
-	while (atomic_read(&data.started) != cpus)
-		cpu_relax();
-
-	if (wait)
-		while (atomic_read(&data.finished) != cpus)
-			cpu_relax();
-	call_data = NULL;
-
-	spin_unlock(&call_lock);
-	return 0;
+	return generic_smp_call_function(func, info, wait, cpu_online_map,
+						send_call_function_ipi);
 }
 EXPORT_SYMBOL(smp_call_function);
 
-- 
1.5.4.GIT

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Ingo Molnar]

* Jens Axboe <jens.axboe@oracle.com> wrote:

> The patch series is also available in the 'generic-ipi' branch from
> 
> git://git.kernel.dk/linux-2.6-block.git
> 
> and the 'io-cpu-affinity' branch is directly based on this.

i'm still wondering about the following fundamental bit: why not use per 
CPU kernel threads? That way you get a fast (lockless) IPI "for free" as 
SMP wakeups already do this.

smp_call_function() is quirky and has deep limitations on atomicity, 
etc., so we are moving away from it and should not base more 
functionality on it.

	Ingo

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

On Fri, Mar 21 2008, Ingo Molnar wrote:
> 
> * Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > The patch series is also available in the 'generic-ipi' branch from
> > 
> > git://git.kernel.dk/linux-2.6-block.git
> > 
> > and the 'io-cpu-affinity' branch is directly based on this.
> 
> i'm still wondering about the following fundamental bit: why not use
> per CPU kernel threads? That way you get a fast (lockless) IPI "for
> free" as SMP wakeups already do this.

The kernel thread variant wont be any more lockless than the
smp_call_function_single() approach, they both have to grab the
destination queue lock. If you recall, I pushed forward on the kernel
thread variant and even still have it online here:

http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=io-cpu-affinity-kthread

which is pretty much identical to io-cpu-affinity, except it uses kernel
threads for completion.

The reason why I dropped the kthread approach is that it was slower.
Time from signal to run was about 33% faster with IPI than with
wake_up_process(). Doing benchmark runs, and the IPI approach won hands
down in cache misses as well.

> smp_call_function() is quirky and has deep limitations on atomicity,
> etc., so we are moving away from it and should not base more
> functionality on it.

The patchset does not build on smp_call_function(), it merely cleans
that stuff up instead of having essentially the same code in each arch.
As more archs are converted, it'll remove lots more code.

The block stuff builds on smp_call_function_single(), which doesn't
suffer from any of the badness that smp_call_function() does.

-- 
Jens Axboe

RE: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Luck, Tony]

> When/if more archs are converted, it'll of course look even better.
> Comments? I've verified that this builts and boots on x86, x86-64
> and powerpc. IA64 may still need a one-liner change, I don't have
> access to that platform anymore so I can't verify that myself. It
> does build, however :-)

Can you give me a clue as to which line needs the one-liner ... although
it builds cleanly for IA64 it panics with a dereference NULL fault. It
looks like we were in the list_add_tail(&data->list, &dst->list) code.

Console log looks like this:

ACPI: Core revision 20070126
Boot processor id 0x0/0xc018
Fixed BSP b0 value from CPU 1
Unable to handle kernel NULL pointer dereference (address 0000000000000000)
swapper[0]: Oops 8804682956800 [1]
Modules linked in:

Pid: 0, CPU 1, comm:              swapper
psr : 00001010084a2010 ifs : 800000000000060e ip  : [<a0000001000be1c0>]    Not tainted (2.6.25-rc6-tiger-smp)
ip is at generic_exec_single+0xc0/0x200
unat: 0000000000000000 pfs : 000000000000060e rsc : 0000000000000003
rnat: 5555555555555555 bsps: 5555555555555555 pr  : 0000000000002655
ldrs: 0000000000000000 ccv : 0000000000000000 fpsr: 0009804c0270433f
csd : 0000000000000000 ssd : 0000000000000000
b0  : a0000001000be180 b6  : e00000007fe131b0 b7  : e00000007fb1bde0
f6  : 000000000000000000000 f7  : 1003e6db6db6db6db6db7
f8  : 1003e00000000000c410a f9  : 1000fffff800000000000
f10 : 1003e000000000001ffff f11 : 000000000000000000000
r1  : a000000100c42f20 r2  : 0000000000000000 r3  : ffffffffffff5968
r8  : 00000010084a6010 r9  : a000000100a617c0 r10 : 0000000000000000
r11 : a000000100a617c0 r12 : e0000001c025fdf0 r13 : e0000001c0250000
r14 : e000000180016058 r15 : e000000180005970 r16 : 0000000000000002
r17 : a000000100a617c0 r18 : 0000000000004000 r19 : a00000010099ffb0
r20 : e0000001c01550c0 r21 : e0000001c01501c0 r22 : 0000000000000000
r23 : e000000180016064 r24 : a07ffffffff20498 r25 : 00000000000000c0
r26 : 00000000000000c1 r27 : 00000010084a6010 r28 : 0000000000000001
r29 : a00000010099ff80 r30 : 0000000000000000 r31 : e000000180005978

Call Trace:
 [<a000000100011bb0>] show_stack+0x50/0xa0
                                sp=e0000001c025f9c0 bsp=e0000001c0250e68
 [<a000000100012490>] show_regs+0x830/0x860
                                sp=e0000001c025fb90 bsp=e0000001c0250e20
 [<a0000001000340a0>] die+0x1c0/0x2c0
                                sp=e0000001c025fb90 bsp=e0000001c0250dd0
 [<a00000010005b150>] ia64_do_page_fault+0x830/0x960
                                sp=e0000001c025fb90 bsp=e0000001c0250d70
 [<a00000010000a600>] ia64_leave_kernel+0x0/0x270
                                sp=e0000001c025fc20 bsp=e0000001c0250d70
 [<a0000001000be1c0>] generic_exec_single+0xc0/0x200
                                sp=e0000001c025fdf0 bsp=e0000001c0250d00
 [<a0000001000be4c0>] generic_smp_call_function_single+0x1c0/0x1e0
                                sp=e0000001c025fdf0 bsp=e0000001c0250cb0
 [<a000000100050aa0>] smp_call_function_single+0x40/0x60
                                sp=e0000001c025fe20 bsp=e0000001c0250c70
 [<a000000100050fa0>] ia64_sync_itc+0xc0/0x3e0
                                sp=e0000001c025fe20 bsp=e0000001c0250c38
 [<a0000001006fbea0>] start_secondary+0x380/0x560
                                sp=e0000001c025fe30 bsp=e0000001c0250be0
 [<a000000100707220>] __kprobes_text_end+0x6c0/0x6f0
                                sp=e0000001c025fe30 bsp=e0000001c0250be0
Kernel panic - not syncing: Attempted to kill the idle task!

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

On Fri, Mar 21 2008, Luck, Tony wrote:
> > When/if more archs are converted, it'll of course look even better.
> > Comments? I've verified that this builts and boots on x86, x86-64
> > and powerpc. IA64 may still need a one-liner change, I don't have
> > access to that platform anymore so I can't verify that myself. It
> > does build, however :-)
> 
> Can you give me a clue as to which line needs the one-liner ... although
> it builds cleanly for IA64 it panics with a dereference NULL fault. It

My thinking was that it may need a bit of help to ensure that the init
function for single call data works,
kernel/smp.c:init_call_single_data() specifically. It may be run before
the arch code has setup all CPU related structures, not sure...

> looks like we were in the list_add_tail(&data->list, &dst->list) code.

Is it data or dst being NULL? Looking at your trace, it must be dst.
Which makes me believe that it is indeed a missing call of the above
function at the right time. So some help would be accepted there :)

-- 
Jens Axboe

RE: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Luck, Tony]

> My thinking was that it may need a bit of help to ensure that the init
> function for single call data works,

The problem may be worse than that.  I see that you have a per-cpu
"call_single_queue" on which you do some list operations.  On ia64
the per-cpu variables live at the same virtual address on each processor
with a per-cpu TLB mapping directing them to a different physical
address on each.  This makes Linux lists very confused.

-Tony

RE: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Luck, Tony]

> The problem may be worse than that.  I see that you have a per-cpu
> "call_single_queue" on which you do some list operations.

Simply hacking in an early call to init_call_single_data() helped
a bit.  I saw a few more kernel messages as all the cpus were
brought up and synchronized ITC with the boot cpu. Then the
system hung hard.  Last messages were:

Total of 16 processors activated (50724.86 BogoMIPS).
net_namespace: 296 bytes
DMI 2.3 present




-Tony

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

On Fri, Mar 21 2008, Luck, Tony wrote:
> > My thinking was that it may need a bit of help to ensure that the init
> > function for single call data works,
> 
> The problem may be worse than that.  I see that you have a per-cpu
> "call_single_queue" on which you do some list operations.  On ia64
> the per-cpu variables live at the same virtual address on each processor
> with a per-cpu TLB mapping directing them to a different physical
> address on each.  This makes Linux lists very confused.

Funky, how does accessing other CPU's per-cpu variables work on ia64
then? Perhaps I made some false assumptions. Unfortunately I have no
access to any IA64 machines, so I either have to yank the ia64 bits
(which is unfortunate, since Alan tests on those :-) or rely on a bit of
help from you and/or others in getting that bit right.

-- 
Jens Axboe

RE: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Luck, Tony]

> Funky, how does accessing other CPU's per-cpu variables work on ia64
> then? Perhaps I made some false assumptions.

The per-cpu memory is mapped at two different spots in the kernel
virtual memory.  When cpuA wants to access per-cpu memory that belongs
to cpuB it can use the mappings that allow access to every percpu
area (which may just be indexing by cpu number into a big block of
memory that has all the per-cpu spaces ... or some more complex
arithmetic and pointers for NUMA systems where the per-cpu memory
ought to be allocated out of memory on the right node for the cpu
that it refers to).

When any cpu wants to access its own per-cpu data, it can do so
via the per-cpu mapping (which is much more efficient from a code
generation perspective at the per-cpu virtual area is the top 64K
of virtual address space, so can be accessed with a small negative
offset from register r0).

This is why lists can be a problem ... since the same memory can
be accessed via two different virtual addresses, things can get
badly knotted when the two different addresses get used in different
parts of the code.  Then operations like "list_empty()" may give
the wrong answer because the virtual address used for head->next
isn't the same as that used for head ... but they both refer to the
same underlying object.

> Unfortunately I have no access to any IA64 machines, so I either
> have to yank the ia64 bits (which is unfortunate, since Alan tests
> on those :-) or rely on a bit of help from you and/or others in
> getting that bit right.

I'll see if I can figure out what is going wrong.

-Tony

RE: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Luck, Tony]

> Funky, how does accessing other CPU's per-cpu variables work on ia64
> then? Perhaps I made some false assumptions.

Having scared you to death with some of the strange weirdness of
ia64 ... it turns out that you made a trivial typo in the ia64
specific part of the patch ... see below.

With this patch and a hack to call init_call_single_data() early
enough the patch boots fine on ia64.

-Tony

commit 8ffe2551f04e55176f7f7935c5a3395cc641d514
Author: Tony Luck <tony.luck@intel.com>
Date:   Mon Mar 24 13:04:11 2008 -0700

    [IA64] Fix typo'd call to generic_smp_call_function_single_interrupt
    
    Should really be calling generic_smp_call_function_interrupt() for
    the IPI_CALL_FUNC case.
    
    Signed-off-by: Tony Luck <tony.luck@intel.com>

diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c
index fa26528..55cbc2c 100644
--- a/arch/ia64/kernel/smp.c
+++ b/arch/ia64/kernel/smp.c
@@ -124,7 +124,7 @@ handle_IPI (int irq, void *dev_id)
 
 			switch (which) {
 			      case IPI_CALL_FUNC:
-				generic_smp_call_function_single_interrupt();
+				generic_smp_call_function_interrupt();
 				break;
 			      case IPI_CALL_FUNC_SINGLE:
 				generic_smp_call_function_single_interrupt();

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Nick Piggin]

On Saturday 22 March 2008 00:15, Jens Axboe wrote:
> On Fri, Mar 21 2008, Ingo Molnar wrote:
> > * Jens Axboe <jens.axboe@oracle.com> wrote:
> > > The patch series is also available in the 'generic-ipi' branch from
> > >
> > > git://git.kernel.dk/linux-2.6-block.git
> > >
> > > and the 'io-cpu-affinity' branch is directly based on this.
> >
> > i'm still wondering about the following fundamental bit: why not use
> > per CPU kernel threads? That way you get a fast (lockless) IPI "for
> > free" as SMP wakeups already do this.
>
> The kernel thread variant wont be any more lockless than the
> smp_call_function_single() approach, they both have to grab the
> destination queue lock. If you recall, I pushed forward on the kernel
> thread variant and even still have it online here:
>
> http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=io-cpu-affinity-kt
>hread
>
> which is pretty much identical to io-cpu-affinity, except it uses kernel
> threads for completion.
>
> The reason why I dropped the kthread approach is that it was slower.
> Time from signal to run was about 33% faster with IPI than with
> wake_up_process(). Doing benchmark runs, and the IPI approach won hands
> down in cache misses as well.

It's obviously going to be faster. The kthread approach needs an IPI
*and* a context switch on the destination CPU.

For broadcast situations, it is also going to be faster, because it
can use platform specific broadcast IPIs 

The only weird atomicity requirements of smp_call_function is due to
its terribly unscalable design. Once you have the producer provide its
own data (or accept -ENOMEM failures), then there is no problem. It's
then even possible to use it from interrupt context with not too much
more work.


> > smp_call_function() is quirky and has deep limitations on atomicity,
> > etc., so we are moving away from it and should not base more
> > functionality on it.
>
> The patchset does not build on smp_call_function(), it merely cleans
> that stuff up instead of having essentially the same code in each arch.
> As more archs are converted, it'll remove lots more code.

Also there is nothing wrong with improving the speed of the existing
implementation. The current smp_call_function is more or less unusable
for any real work because it is totally serialised.

Thanks for taking this patchset under your wing, Jens.

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

On Mon, Mar 24 2008, Luck, Tony wrote:
> > Funky, how does accessing other CPU's per-cpu variables work on ia64
> > then? Perhaps I made some false assumptions.
> 
> Having scared you to death with some of the strange weirdness of
> ia64 ... it turns out that you made a trivial typo in the ia64
> specific part of the patch ... see below.
> 
> With this patch and a hack to call init_call_single_data() early
> enough the patch boots fine on ia64.
> 
> -Tony
> 
> commit 8ffe2551f04e55176f7f7935c5a3395cc641d514
> Author: Tony Luck <tony.luck@intel.com>
> Date:   Mon Mar 24 13:04:11 2008 -0700
> 
>     [IA64] Fix typo'd call to generic_smp_call_function_single_interrupt
>     
>     Should really be calling generic_smp_call_function_interrupt() for
>     the IPI_CALL_FUNC case.
>     
>     Signed-off-by: Tony Luck <tony.luck@intel.com>
> 
> diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c
> index fa26528..55cbc2c 100644
> --- a/arch/ia64/kernel/smp.c
> +++ b/arch/ia64/kernel/smp.c
> @@ -124,7 +124,7 @@ handle_IPI (int irq, void *dev_id)
>  
>  			switch (which) {
>  			      case IPI_CALL_FUNC:
> -				generic_smp_call_function_single_interrupt();
> +				generic_smp_call_function_interrupt();
>  				break;

Doh, that was a pretty silly typo. Thanks, I've merged it with the
patch!

So now I/we just need to figure out why the hack to call
init_call_single_data() is needed. You seem to imply it was being called
too late, I thought perhaps too early. Where did you stick the
init_call_single_data() call in?

-- 
Jens Axboe

RE: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Luck, Tony]

> So now I/we just need to figure out why the hack to call
> init_call_single_data() is needed. You seem to imply it was being called
> too late, I thought perhaps too early. Where did you stick the
> init_call_single_data() call in?

In ia64 the first calls to smp_call_function_single() are made
while bringing up other cpus ... which happens from:

	kernel_init()
		smp_init()

The init calls are made a few lines later (still in kernel_init):

		do_basic_setup()
			do_initcalls()

I moved the call radically earlier (before sched_init() in
init/main.c:start_kernel()) just to be sure, but that was
overkill.

Perhaps making the call from do_pre_smp_initcalls() is the
logical place?  Like this (though purists will say that the
extern declaration should be in some header file):

Signed-off-by: Tony Luck <tony.luck@intel.com>

diff --git a/init/main.c b/init/main.c
index 99ce949..0c140ed 100644
--- a/init/main.c
+++ b/init/main.c
@@ -750,7 +750,9 @@ __setup("nosoftlockup", nosoftlockup_setup);
 static void __init do_pre_smp_initcalls(void)
 {
 	extern int spawn_ksoftirqd(void);
+	extern int __cpuinit init_call_single_data(void);
 
+	init_call_single_data();
 	migration_init();
 	spawn_ksoftirqd();
 	if (!nosoftlockup)
diff --git a/kernel/smp.c b/kernel/smp.c
index ab2da5c..df1b651 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -40,7 +40,7 @@ struct call_single_queue {
 static struct call_function_data call_data_fallback;
 static unsigned long call_fallback_used;
 
-static int __cpuinit init_call_single_data(void)
+int __cpuinit init_call_single_data(void)
 {
 	int i;
 
@@ -53,7 +53,6 @@ static int __cpuinit init_call_single_data(void)
 	}
 	return 0;
 }
-core_initcall(init_call_single_data);
 
 /*
  * Insert a previously allocated call_single_data element for execution

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

On Tue, Mar 25 2008, Luck, Tony wrote:
> > So now I/we just need to figure out why the hack to call
> > init_call_single_data() is needed. You seem to imply it was being called
> > too late, I thought perhaps too early. Where did you stick the
> > init_call_single_data() call in?
> 
> In ia64 the first calls to smp_call_function_single() are made
> while bringing up other cpus ... which happens from:
> 
> 	kernel_init()
> 		smp_init()
> 
> The init calls are made a few lines later (still in kernel_init):
> 
> 		do_basic_setup()
> 			do_initcalls()
> 
> I moved the call radically earlier (before sched_init() in
> init/main.c:start_kernel()) just to be sure, but that was
> overkill.
> 
> Perhaps making the call from do_pre_smp_initcalls() is the
> logical place?  Like this (though purists will say that the
> extern declaration should be in some header file):

It looks fine to me, not a big deal I think... I was woried that
core_initcall() would not suit all, so this helps.

Thanks Tony!

-- 
Jens Axboe

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Ingo Molnar]

* Jens Axboe <jens.axboe@oracle.com> wrote:

> which is pretty much identical to io-cpu-affinity, except it uses 
> kernel threads for completion.
> 
> The reason why I dropped the kthread approach is that it was slower. 
> Time from signal to run was about 33% faster with IPI than with 
> wake_up_process(). Doing benchmark runs, and the IPI approach won 
> hands down in cache misses as well.

with irq threads we'll have all irq context run in kthread context 
again. Could you show me how you measured the performance of the kthread 
approach versus the raw-IPI approach?

we can do a million kthread context switches per CPU per second, so 
kthread context-switch cost cannot be a true performance limit, unless 
you micro-benchmarked this.

	Ingo

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

On Thu, Mar 27 2008, Ingo Molnar wrote:
> 
> * Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > which is pretty much identical to io-cpu-affinity, except it uses 
> > kernel threads for completion.
> > 
> > The reason why I dropped the kthread approach is that it was slower. 
> > Time from signal to run was about 33% faster with IPI than with 
> > wake_up_process(). Doing benchmark runs, and the IPI approach won 
> > hands down in cache misses as well.
> 
> with irq threads we'll have all irq context run in kthread context 
> again. Could you show me how you measured the performance of the kthread 
> approach versus the raw-IPI approach?

There were 3 different indicators that the irq thread approach was
slower:

- Time from signal to actual run of the trigger was ~2usec vs ~3usec for
  IPI vs kthread. That was a microbenchmark.
- Cache misses were higher with the kthread approach.
- Actual performance in non-micro benchmarks was lower with the kthread
  approach.

I'll defer to Alan for the actual numbers, most of this was done in
private mails back and forth doing performance analysis. The initial
testing was done with the IPI hack, then we moved to the kthread
approach. Later the two were pitted against each other and the kthread
part was definitely slower. It ended up using more system time than the
IPI approach. So the kthread approach was than abandoned and all testing
has been on the smp_call_function_single() branch since then.

I very much wanted the kthread approach to work, since it's easier to
work with. It's not for lack of will or trying... I'll be happy to
supply you otherwise identical patches for this, the only difference
being kthread of IPI completions if you want to play with this.

> we can do a million kthread context switches per CPU per second, so 
> kthread context-switch cost cannot be a true performance limit, unless 
> you micro-benchmarked this.

At which point you wont be doing much else, so a cs microbenchmark is
not really that interesting.

-- 
Jens Axboe

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Ingo Molnar]

* Jens Axboe <jens.axboe@oracle.com> wrote:

> I very much wanted the kthread approach to work, since it's easier to 
> work with. It's not for lack of will or trying... I'll be happy to 
> supply you otherwise identical patches for this, the only difference 
> being kthread of IPI completions if you want to play with this.

i'd love to be able to run/pull something simple that enables me to 
replicate the measurements you did on a generic PC [without having to 
hit any real IO hardware which would put any context switching effects 
down into the noise category].

Obviously since the kthread approach embedds IPI sending it can never be 
as fast as a pure IPI approach - but it should still be reasonably fast. 
3 usecs versus 2 usecs in a microbenchmark sounds about right to me, but 
it would be better to make it a better-sounding 2.5 usecs versus 2 usecs 
or so :-)

	Ingo

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

On Thu, Mar 27 2008, Ingo Molnar wrote:
> 
> * Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > I very much wanted the kthread approach to work, since it's easier to 
> > work with. It's not for lack of will or trying... I'll be happy to 
> > supply you otherwise identical patches for this, the only difference 
> > being kthread of IPI completions if you want to play with this.
> 
> i'd love to be able to run/pull something simple that enables me to 
> replicate the measurements you did on a generic PC [without having to 
> hit any real IO hardware which would put any context switching effects 
> down into the noise category].

You can pull io-cpu-affinity or io-cpu-affinity-kthread from
git://git.kernel.dk/linux-2.6-block.git - or just see the two attached
patches, apply either one to current -git to test it.

> Obviously since the kthread approach embedds IPI sending it can never be 
> as fast as a pure IPI approach - but it should still be reasonably fast. 
> 3 usecs versus 2 usecs in a microbenchmark sounds about right to me, but 
> it would be better to make it a better-sounding 2.5 usecs versus 2 usecs 
> or so :-)

If you have ideas to speedup the kthread approach, fire away :-)

-- 
Jens Axboe


[-- Attachment #2: io-affinity-ipi.patch.bz2 --]
[-- Type: application/x-bzip, Size: 15107 bytes --]

[-- Attachment #3: io-affinity-kthread.patch.bz2 --]
[-- Type: application/x-bzip, Size: 6966 bytes --]

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Ingo Molnar]

* Jens Axboe <jens.axboe@oracle.com> wrote:

> > i'd love to be able to run/pull something simple that enables me to 
> > replicate the measurements you did on a generic PC [without having 
> > to hit any real IO hardware which would put any context switching 
> > effects down into the noise category].
> 
> You can pull io-cpu-affinity or io-cpu-affinity-kthread from 
> git://git.kernel.dk/linux-2.6-block.git - or just see the two attached 
> patches, apply either one to current -git to test it.

another stupid question: what should i run in user-space to replicate 
your "3 usecs versus 2 usecs" result? io-affinity-ipi.patch seems to 
have no self-benchmarking capability at first sight. (I'd rather not try 
and cook up anything myself - i'd like to reproduce the workload you 
think is relevant for your IO affinity purposes.) Best would be to have 
a Kconfig based self-test that just runs during bootup if i boot a 
bzImage. (laziness rules - and this way i could also track performance 
regressions more easily, by looking at historic serial logs.)

	Ingo

Re: [PATCH 0/5] Generic smp_call_function(), improvements, and smp_call_function_single()

[Jens Axboe]

On Thu, Mar 27 2008, Ingo Molnar wrote:
> 
> * Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > > i'd love to be able to run/pull something simple that enables me to 
> > > replicate the measurements you did on a generic PC [without having 
> > > to hit any real IO hardware which would put any context switching 
> > > effects down into the noise category].
> > 
> > You can pull io-cpu-affinity or io-cpu-affinity-kthread from 
> > git://git.kernel.dk/linux-2.6-block.git - or just see the two attached 
> > patches, apply either one to current -git to test it.
> 
> another stupid question: what should i run in user-space to replicate 
> your "3 usecs versus 2 usecs" result? io-affinity-ipi.patch seems to 
> have no self-benchmarking capability at first sight. (I'd rather not try 
> and cook up anything myself - i'd like to reproduce the workload you 
> think is relevant for your IO affinity purposes.) Best would be to have 
> a Kconfig based self-test that just runs during bootup if i boot a 
> bzImage. (laziness rules - and this way i could also track performance 
> regressions more easily, by looking at historic serial logs.)

I didn't do those numbers - Alan, when you timed the kthread vs ipi
"wake up", what did you use? I'm guessing some hacked in test, perhaps
you can pass that to Ingo?

-- 
Jens Axboe