[PATCH] smp: allow calling smp_call_function_single from softirq

[Max Filippov]

This prevents the following deadlocks on the sending CPU by eliminating
interrupts between the point where CSD is locked and IPI is sent to peer
CPU.

Case 1:
 CPU 0
 smp_call_function_single(CPU 1, wait = 0)
     csd_lock(CPU 0)
     irq_enter()
     irq_exit()
     __do_softirq()
     smp_call_function_single(CPU 1, wait = 0)
       csd_lock(CPU 0) => deadlock, as csd will never be unlocked

Case 2:
 CPU 0
 smp_call_function_single(CPU 1, wait = 1)
     csd_lock(on stack)
     queue csd to CPU 1 call_single_queue
     irq_enter()
     irq_exit()
     __do_softirq()
     smp_call_function_single(CPU 1, wait = 1)
       setup csd (on stack)
       queue csd to CPU 1 call_single_queue
       csd_lock_wait => never returns, as IPI was never sent to CPU 1

Signed-off-by: Max Filippov <jcmvbkbc@xxxxxxxxx>
---
 kernel/smp.c | 47 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index 0564571..7bc9a01 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -122,6 +122,30 @@ static void csd_lock(struct call_single_data *csd)
 	smp_mb();
 }

+static unsigned long csd_lock_irqsave(struct call_single_data *csd)
+{
+	unsigned long flags;
+
+	for (;;) {
+		csd_lock_wait(csd);
+		local_irq_save(flags);
+		if (csd->flags & CSD_FLAG_LOCK)
+			local_irq_restore(flags);
+		else
+			break;
+	}
+	csd->flags |= CSD_FLAG_LOCK;
+
+	/*
+	 * prevent CPU from reordering the above assignment
+	 * to ->flags with any subsequent assignments to other
+	 * fields of the specified call_single_data structure:
+	 */
+	smp_mb();
+
+	return flags;
+}
+
 static void csd_unlock(struct call_single_data *csd)
 {
 	WARN_ON(!(csd->flags & CSD_FLAG_LOCK));
@@ -140,16 +164,20 @@ static void csd_unlock(struct call_single_data *csd)
  * ->func, ->info, and ->flags set.
  */
 static
-void generic_exec_single(int cpu, struct call_single_data *csd, int wait)
+void generic_exec_single(int cpu, struct call_single_data *csd,
+			 smp_call_func_t func, void *info, int wait)
 {
 	struct call_single_queue *dst = &per_cpu(call_single_queue, cpu);
-	unsigned long flags;
+	unsigned long flags = csd_lock_irqsave(csd);
 	int ipi;

-	raw_spin_lock_irqsave(&dst->lock, flags);
+	csd->func = func;
+	csd->info = info;
+
+	raw_spin_lock(&dst->lock);
 	ipi = list_empty(&dst->list);
 	list_add_tail(&csd->list, &dst->list);
-	raw_spin_unlock_irqrestore(&dst->lock, flags);
+	raw_spin_unlock(&dst->lock);

 	/*
 	 * The list addition should be visible before sending the IPI
@@ -165,6 +193,8 @@ void generic_exec_single(int cpu, struct call_single_data *csd, int wait)
 	if (ipi)
 		arch_send_call_function_single_ipi(cpu);

+	local_irq_restore(flags);
+
 	if (wait)
 		csd_lock_wait(csd);
 }
@@ -245,11 +275,7 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
 			if (!wait)
 				csd = &__get_cpu_var(csd_data);

-			csd_lock(csd);
-
-			csd->func = func;
-			csd->info = info;
-			generic_exec_single(cpu, csd, wait);
+			generic_exec_single(cpu, csd, func, info, wait);
 		} else {
 			err = -ENXIO;	/* CPU not online */
 		}
@@ -335,8 +361,7 @@ void __smp_call_function_single(int cpu, struct call_single_data *csd,
 		csd->func(csd->info);
 		local_irq_restore(flags);
 	} else {
-		csd_lock(csd);
-		generic_exec_single(cpu, csd, wait);
+		generic_exec_single(cpu, csd, csd->func, csd->info, wait);
 	}
 	put_cpu();
 }
-- 
1.8.1.4


-- 
Thanks.
-- Max
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/