> -----Original Message----- > From: Lai Jiangshan [mailto:eag0...@gmail.com] > Sent: Wednesday, February 27, 2013 10:51 PM > To: Liu, Chuansheng > Cc: mi...@kernel.org; pet...@infradead.org; jbeul...@suse.com; > paul...@linux.vnet.ibm.com; a...@linux-foundation.org; > min...@mina86.org; srivatsa.b...@linux.vnet.ibm.com; > linux-kernel@vger.kernel.org; Zhang, Jun; Wu, Fengguang > Subject: Re: [PATCH V2] smp: Give WARN()ing when calling > smp_call_function_many()/single() in serving irq > > On Sat, Feb 16, 2013 at 10:10 PM, Chuansheng Liu > <chuansheng....@intel.com> wrote: > > 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. > > The code smells bad. in_serving_softirq() don't take spin_lock_bh() in > account. > > CPUA CPUB CPUC > spin_lock(&lockA) > Any irq coming, call > the irq handler > irq_exit() > spin_lock_irq(&lockA) > *Blocking* here > due to CPUB hold it > spin_lock_bh(&lockB) > __do_softirq() > run_timer_softirq() > 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 > And add: > > Reviewed-by: Lai Jiangshan <la...@cn.fujitsu.com> -- 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/
