From c5cae661d6cf808b6984762f763261adf35f3eb7 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 17 Dec 2009 13:57:09 +0000 Subject: xen: fix hang on suspend. In 65f63384 "xen: improve error handling in do_suspend" I said: - xs_suspend()/xs_resume() and dpm_suspend_noirq()/dpm_resume_noirq() were not nested in the obvious way. and changed the ordering of the calls as so: BEFORE AFTER xs_suspend dpm_suspend_noirq dpm_suspend_noirq xs_suspend *SUSPEND* *SUSPEND* dpm_resume_noirq dpm_resume_noirq xs_resume xs_resume Clearly this is not an improvement and I was talking rubbish. In particular the new ordering is susceptible to a hang if a xenstore write is in progress at the point at which the suspend kicks in. When the suspend process calls xs_suspend it tries to take the request_mutex but if a write is in progress it could be looping in xenbus_xs.c:read_reply() waiting for something to arrive on &xs_state.reply_list while holding the request_mutex (taken in the caller of read_reply). However if we have done dpm_suspend_noirq before xs_suspend then we won't get any more xenstore interrupts and process_msg() will never be woken up to add anything to the reply_list. Fix this by calling xs_suspend before dpm_suspend_noirq. If dpm_suspend_noirq fails then make sure we go through the xs_suspend_cancel() code path. Signed-off-by: Ian Campbell Acked-by: Jeremy Fitzhardinge Cc: Stable Kernel --- drivers/xen/manage.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c index c4997930afc..5d42d55e299 100644 --- a/drivers/xen/manage.c +++ b/drivers/xen/manage.c @@ -102,15 +102,15 @@ static void do_suspend(void) goto out_thaw; } + printk(KERN_DEBUG "suspending xenstore...\n"); + xs_suspend(); + err = dpm_suspend_noirq(PMSG_SUSPEND); if (err) { printk(KERN_ERR "dpm_suspend_noirq failed: %d\n", err); goto out_resume; } - printk(KERN_DEBUG "suspending xenstore...\n"); - xs_suspend(); - err = stop_machine(xen_suspend, &cancelled, cpumask_of(0)); dpm_resume_noirq(PMSG_RESUME); @@ -120,13 +120,13 @@ static void do_suspend(void) cancelled = 1; } +out_resume: if (!cancelled) { xen_arch_resume(); xs_resume(); } else xs_suspend_cancel(); -out_resume: dpm_resume_end(PMSG_RESUME); /* Make sure timer events get retriggered on all CPUs */ -- cgit From ca4dbc668412d5fe039be3e26e8e717a616d1ca5 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Wed, 17 Feb 2010 18:49:54 -0800 Subject: xen: Remove unnecessary arch specific xen irq functions. Right now xen's use of the x86 and ia64 handle_irq is just bizarre and very fragile as it is very non-obvious the function exists and is is used by code out in drivers/.... Luckily using handle_irq is completely unnecessary, and we can just use the generic irq apis instead. This still leaves drivers/xen/events.c as a problematic user of the generic irq apis it has "static struct irq_info irq_info[NR_IRQS]" but that can be fixed some other time. Signed-off-by: Eric W. Biederman LKML-Reference: <4B7CAAD2.10803@kernel.org> Acked-by: Jeremy Fitzhardinge Cc: Ian Campbell Signed-off-by: H. Peter Anvin --- drivers/xen/events.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index ce602dd09bc..2f8413794d0 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -649,9 +649,13 @@ void xen_evtchn_do_upcall(struct pt_regs *regs) int bit_idx = __ffs(pending_bits); int port = (word_idx * BITS_PER_LONG) + bit_idx; int irq = evtchn_to_irq[port]; + struct irq_desc *desc; - if (irq != -1) - handle_irq(irq, regs); + if (irq != -1) { + desc = irq_to_desc(irq); + if (desc) + generic_handle_irq_desc(irq, desc); + } } } -- cgit