From: "Liu, Chuansheng" <chuansheng.liu@intel.com>
To: Lai Jiangshan <eag0628@gmail.com>
Cc: "mingo@kernel.org" <mingo@kernel.org>,
"peterz@infradead.org" <peterz@infradead.org>,
"jbeulich@suse.com" <jbeulich@suse.com>,
"paulmck@linux.vnet.ibm.com" <paulmck@linux.vnet.ibm.com>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"mina86@mina86.org" <mina86@mina86.org>,
"srivatsa.bhat@linux.vnet.ibm.com"
<srivatsa.bhat@linux.vnet.ibm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Zhang, Jun" <jun.zhang@intel.com>,
"Wu, Fengguang" <fengguang.wu@intel.com>
Subject: RE: [PATCH V2] smp: Give WARN()ing when calling smp_call_function_many()/single() in serving irq
Date: Fri, 1 Mar 2013 03:37:11 +0000 [thread overview]
Message-ID: <27240C0AC20F114CBF8149A2696CBE4A2427D4@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <CACvQF51cfqYk3idypNrQ54TOYJSPYvzR+1TTRBOQ5_vH8etwWw@mail.gmail.com>
> -----Original Message-----
> From: Lai Jiangshan [mailto:eag0628@gmail.com]
> Sent: Wednesday, February 27, 2013 10:51 PM
> To: Liu, Chuansheng
> Cc: mingo@kernel.org; peterz@infradead.org; jbeulich@suse.com;
> paulmck@linux.vnet.ibm.com; akpm@linux-foundation.org;
> mina86@mina86.org; srivatsa.bhat@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.liu@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 <laijs@cn.fujitsu.com>
next prev parent reply other threads:[~2013-03-01 3:37 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-06 15:18 [PATCH] smp: give WARN in case of in_interrupt() when calling smp_call_function_many/single Chuansheng Liu
2013-02-06 13:42 ` [tip:core/locking] smp: Give WARN()ing if in_interrupt() when calling smp_call_function_many()/single() tip-bot for Chuansheng Liu
2013-02-11 12:20 ` [tip:core/locking] Revert "smp: Give WARN()ing if in_interrupt() when calling smp_call_function_many()/single()" tip-bot for Ingo Molnar
2013-02-16 5:26 ` Liu, Chuansheng
2013-02-16 13:57 ` [PATCH] smp: Give WARN()ing when calling smp_call_function_many()/single() in serving irq Chuansheng Liu
2013-02-16 14:10 ` [PATCH V2] " Chuansheng Liu
2013-02-18 1:38 ` Fengguang Wu
2013-02-19 23:01 ` Andrew Morton
2013-02-20 1:06 ` Fengguang Wu
2013-02-20 1:22 ` Liu, Chuansheng
2013-02-27 14:50 ` Lai Jiangshan
2013-03-01 3:37 ` Liu, Chuansheng [this message]
2013-08-05 22:46 ` Andrew Morton
2013-07-05 13:50 ` Thomas Gleixner
2013-07-05 14:37 ` Thomas Gleixner
2013-07-07 3:59 ` Wang YanQing
2013-07-07 13:47 ` Thomas Gleixner
2013-12-06 1:29 ` Max Filippov
2013-12-06 14:02 ` Thomas Gleixner
2013-12-06 18:31 ` Max Filippov
2013-07-07 2:41 ` Wang YanQing
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=27240C0AC20F114CBF8149A2696CBE4A2427D4@SHSMSX101.ccr.corp.intel.com \
--to=chuansheng.liu@intel.com \
--cc=akpm@linux-foundation.org \
--cc=eag0628@gmail.com \
--cc=fengguang.wu@intel.com \
--cc=jbeulich@suse.com \
--cc=jun.zhang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mina86@mina86.org \
--cc=mingo@kernel.org \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=srivatsa.bhat@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).