On Fri, 1 Mar 2013 03:37:11 +0000 "Liu, Chuansheng" <chuansheng....@intel.com> wrote:
> > > spin_lock_bh(&lockB) > > *Blocking* heredue to > > CPUC hold it > > call > > smp_call_function_many() > > send IPI > > interrupt to CPUA > > > > wait_csd() > > > > *Blocking* here. > > > > So it is still deadlock. but your code does not warn it. > In your case, even you change spin_lock_bh() to spin_lock(), the deadlock is > still there. So no relation with _bh() at all, > Do not need warning for such deadlock case in smp_call_xxx() or for _bh() > case. > > > so in_softirq() is better than in_serving_softirq() in in_serving_irq(), > > and results in_serving_irq() is the same as in_interrupt(). > > > > so please remove in_serving_irq() and use in_interrupt() instead. > The original patch is using in_interrupt(). https://lkml.org/lkml/2013/2/6/34 > (ancient thread) It's not clear (to me) that all these issues are settled. Can we please take another look at this? The patch has been in -mm and linux-next for five months with no issues. But as far as I know, it hasn't detected any kernel bugs, so perhaps we just don't need it? From: Chuansheng Liu <chuansheng....@intel.com> Subject: smp: give WARN()ing when calling smp_call_function_many()/single() in serving irq Currently the functions smp_call_function_many()/single() will give a WARN()ing only in the case of irqs_disabled(), but that check is not enough to guarantee execution of the SMP cross-calls. In many other cases such as softirq handling/interrupt handling, the two APIs still can not be called, just as the smp_call_function_many() comments say: * You must not call this function with disabled interrupts or from a * hardware interrupt handler or from a bottom half handler. Preemption * must be disabled when calling this function. There is a real case for softirq DEADLOCK case: CPUA CPUB spin_lock(&spinlock) Any irq coming, call the irq handler irq_exit() spin_lock_irq(&spinlock) <== Blocking here due to CPUB hold it __do_softirq() run_timer_softirq() timer_cb() call smp_call_function_many() send IPI interrupt to CPUA wait_csd() Then both CPUA and CPUB will be deadlocked here. So we should give a warning in the nmi, hardirq or softirq context as well. Moreover, adding one new macro in_serving_irq() which indicates we are processing nmi, hardirq or sofirq. Signed-off-by: liu chuansheng <chuansheng....@intel.com> Cc: Ingo Molnar <mi...@elte.hu> Cc: Peter Zijlstra <a.p.zijls...@chello.nl> Tested-by: Fengguang Wu <fengguang...@intel.com> Cc: Lai Jiangshan <eag0...@gmail.com> Signed-off-by: Andrew Morton <a...@linux-foundation.org> --- include/linux/hardirq.h | 5 +++++ kernel/smp.c | 11 +++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff -puN include/linux/hardirq.h~smp-give-warning-when-calling-smp_call_function_many-single-in-serving-irq include/linux/hardirq.h --- a/include/linux/hardirq.h~smp-give-warning-when-calling-smp_call_function_many-single-in-serving-irq +++ a/include/linux/hardirq.h @@ -94,6 +94,11 @@ */ #define in_nmi() (preempt_count() & NMI_MASK) +/* + * Are we in nmi,irq context, or softirq context? + */ +#define in_serving_irq() (in_nmi() || in_irq() || in_serving_softirq()) + #if defined(CONFIG_PREEMPT_COUNT) # define PREEMPT_CHECK_OFFSET 1 #else diff -puN kernel/smp.c~smp-give-warning-when-calling-smp_call_function_many-single-in-serving-irq kernel/smp.c --- a/kernel/smp.c~smp-give-warning-when-calling-smp_call_function_many-single-in-serving-irq +++ a/kernel/smp.c @@ -12,6 +12,7 @@ #include <linux/gfp.h> #include <linux/smp.h> #include <linux/cpu.h> +#include <linux/hardirq.h> #include "smpboot.h" @@ -243,8 +244,9 @@ int smp_call_function_single(int cpu, sm * send smp call function interrupt to this cpu and as such deadlocks * can't happen. */ - WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled() - && !oops_in_progress); + WARN_ON_ONCE(cpu_online(this_cpu) + && (irqs_disabled() || in_serving_irq()) + && !oops_in_progress); if (cpu == this_cpu) { local_irq_save(flags); @@ -381,8 +383,9 @@ void smp_call_function_many(const struct * send smp call function interrupt to this cpu and as such deadlocks * can't happen. */ - WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled() - && !oops_in_progress && !early_boot_irqs_disabled); + WARN_ON_ONCE(cpu_online(this_cpu) + && (irqs_disabled() || in_serving_irq()) + && !oops_in_progress && !early_boot_irqs_disabled); /* Try to fastpath. So, what's a CPU they want? Ignoring this one. */ cpu = cpumask_first_and(mask, cpu_online_mask); _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
