From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH 1/2] xen: allow on_selected_cpus with interrupts disabled Date: Fri, 03 May 2013 16:57:31 +0100 Message-ID: References: <1367594468.28742.121.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1367594468.28742.121.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Stefano Stabellini Cc: Julien Grall , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 03/05/2013 16:21, "Ian Campbell" wrote: > On Fri, 2013-05-03 at 15:58 +0100, Stefano Stabellini wrote: >> Allow on_selected_cpus with interrupts disabled, use it with care. > > This is a deadlock waiting to happen. Can we not find a way to do cross > CPU EOI without it? If we can guarantee that we only need to EOI on one > CPU then does that make a specialised SGI vector more plausible? > > Can the IPI call not be moved outside the lock? i.e. remove it from the > list under the lock and then IPI outside? > > Or could you queue the IRQ on a per-pcpu list of IRQs to EOI and then > outside the lock send an IPI to the other CPU to check the list. > > At the least this should assert that he current cpu isn't in the mask > when wait == 1. There's little chance of me being flexible on changing the on_selected_cpus() interface. This may be better handled under arch/arm, or with a new interface, or just as you sugegst rethinking the higher-level problem so you don't get painted into this corner in the first place. >> Signed-off-by: Stefano Stabellini >> --- >> xen/common/smp.c | 7 +++---- >> 1 files changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/xen/common/smp.c b/xen/common/smp.c >> index dcd93ad..7deb97c 100644 >> --- a/xen/common/smp.c >> +++ b/xen/common/smp.c >> @@ -33,10 +33,9 @@ void on_selected_cpus( >> int wait) >> { >> unsigned int nr_cpus; >> + unsigned long flags; >> >> - ASSERT(local_irq_is_enabled()); >> - >> - spin_lock(&call_lock); >> + spin_lock_irqsave(&call_lock, flags); >> >> cpumask_copy(&call_data.selected, selected); >> >> @@ -54,7 +53,7 @@ void on_selected_cpus( >> cpu_relax(); >> >> out: >> - spin_unlock(&call_lock); >> + spin_unlock_irqrestore(&call_lock, flags); >> } >> >> void smp_call_function_interrupt(void) > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel