Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   luferry@163.com
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Rik van Riel <riel@surriel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        linux-kernel@vger.kernel.org, luferry <luferry@163.com>
Subject: [PATCH v2] smp: avoid generic_exec_single cause system lockup
Date:   Thu, 18 Jul 2019 16:03:08 +0800
Message-Id: <20190718080308.48381-1-luferry@163.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

From: luferry <luferry@163.com>

The race can reproduced by sending wait enabled IPI in softirq/irq env

src cpu only send ipi when dst cpu with queue empty, if interrupts
disturbed between llist_add and send_ipi. Interrupt handler may raise
softirq.In irq env, if src cpu try send_ipi to same dst cpu with
wait enabled. Since dst cpu's queue is not empty, src cpu won't send
ipi and dst cpu won't be waked up. src cpu will stall in
csd_lock_wait(csd). Which may cause soft lockup or hard lockup depends on
which time other cpus do send IPI to dst cpu.

So just send IPI when wait enabled and in_interrupt()

if (llist_add(&csd->llist, &per_cpu(call_single_queue, cpu)))
	// src cpu got interrupt here
     arch_send_call_function_single_ipi(cpu);

CPU0                                   CPU1

kernel env:smp_call_function         call_single_queue empty
kernel env:llist_add
                                       call_single_queue got csd
get interrupt
raise softirq
irq env:smp_call_function with wait
irq env:llist_add
irq env:queue not empty and skip send ipi
irq env:waiting for csd execution

Signed-off-by: luferry <luferry@163.com>
---
 kernel/smp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index d155374632eb..5f5343e17bb3 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -142,9 +142,8 @@ static DEFINE_PER_CPU_SHARED_ALIGNED(call_single_data_t, csd_data);
 static int generic_exec_single(int cpu, call_single_data_t *csd,
 			       smp_call_func_t func, void *info)
 {
+	unsigned long flags;
 	if (cpu == smp_processor_id()) {
-		unsigned long flags;
-
 		/*
 		 * We can unlock early even for the synchronous on-stack case,
 		 * since we're doing this from the same CPU..
@@ -176,8 +175,10 @@ static int generic_exec_single(int cpu, call_single_data_t *csd,
 	 * locking and barrier primitives. Generic code isn't really
 	 * equipped to do the right thing...
 	 */
+	local_irq_save(flags);
 	if (llist_add(&csd->llist, &per_cpu(call_single_queue, cpu)))
 		arch_send_call_function_single_ipi(cpu);
+	local_irq_restore(flags);
 
 	return 0;
 }
@@ -404,6 +405,7 @@ EXPORT_SYMBOL_GPL(smp_call_function_any);
 void smp_call_function_many(const struct cpumask *mask,
 			    smp_call_func_t func, void *info, bool wait)
 {
+	unsigned long flags;
 	struct call_function_data *cfd;
 	int cpu, next_cpu, this_cpu = smp_processor_id();
 
@@ -446,6 +448,8 @@ void smp_call_function_many(const struct cpumask *mask,
 		return;
 
 	cpumask_clear(cfd->cpumask_ipi);
+
+	local_irq_save(flags);
 	for_each_cpu(cpu, cfd->cpumask) {
 		call_single_data_t *csd = per_cpu_ptr(cfd->csd, cpu);
 
@@ -460,6 +464,7 @@ void smp_call_function_many(const struct cpumask *mask,
 
 	/* Send a message to all CPUs in the map */
 	arch_send_call_function_ipi_mask(cfd->cpumask_ipi);
+	local_irq_restore(flags);
 
 	if (wait) {
 		for_each_cpu(cpu, cfd->cpumask) {
-- 
2.14.1.40.g8e62ba1