[PATCHv1] xen/events/fifo: Handle linked events when closing a PIRQ port

* [PATCHv1] xen/events/fifo: Handle linked events when closing a PIRQ port
@ 2015-08-10 14:24 David Vrabel
  2015-08-10 16:09 ` Boris Ostrovsky
  2015-08-10 16:47 ` linux
  0 siblings, 2 replies; 7+ messages in thread
From: David Vrabel @ 2015-08-10 14:24 UTC (permalink / raw)
  To: xen-devel
  Cc: Ross Lagerwall, Boris Ostrovsky, Sander Eikelenboom, David Vrabel

Commit fcdf31a7c162de0c93a2bee51df4688ab0a348f8 (xen/events/fifo:
Handle linked events when closing a port) did not handle closing a
port bound to a PIRQ because these are closed from shutdown_pirq()
which is called with interrupts disabled.

Defer the close to a work queue where we can safely spin waiting for
the LINKED bit to clear.  For simplicity, the close is always deferred
even if it is not required (i.e., we're already in process context).

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
---
Cc: Sander Eikelenboom <linux@eikelenboom.it>
---
 drivers/xen/events/events_2l.c       | 10 +++++++
 drivers/xen/events/events_base.c     | 13 +--------
 drivers/xen/events/events_fifo.c     | 52 +++++++++++++++++++++++++++---------
 drivers/xen/events/events_internal.h |  5 ++--
 4 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/drivers/xen/events/events_2l.c b/drivers/xen/events/events_2l.c
index 7dd4631..82c90de 100644
--- a/drivers/xen/events/events_2l.c
+++ b/drivers/xen/events/events_2l.c
@@ -354,6 +354,15 @@ static void evtchn_2l_resume(void)
 				EVTCHN_2L_NR_CHANNELS/BITS_PER_EVTCHN_WORD);
 }
 
+static void evtchn_2l_close(unsigned int port, unsigned int cpu)
+{
+	struct evtchn_close close;
+
+	close.port = port;
+	if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
+		BUG();
+}
+
 static const struct evtchn_ops evtchn_ops_2l = {
 	.max_channels      = evtchn_2l_max_channels,
 	.nr_channels       = evtchn_2l_max_channels,
@@ -366,6 +375,7 @@ static const struct evtchn_ops evtchn_ops_2l = {
 	.unmask            = evtchn_2l_unmask,
 	.handle_events     = evtchn_2l_handle_events,
 	.resume	           = evtchn_2l_resume,
+	.close             = evtchn_2l_close,
 };
 
 void __init xen_evtchn_2l_init(void)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 1495ecc..e3f0049 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -452,17 +452,6 @@ static void xen_free_irq(unsigned irq)
 	irq_free_desc(irq);
 }
 
-static void xen_evtchn_close(unsigned int port, unsigned int cpu)
-{
-	struct evtchn_close close;
-
-	xen_evtchn_op_close(port, cpu);
-
-	close.port = port;
-	if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
-		BUG();
-}
-
 static void pirq_query_unmask(int irq)
 {
 	struct physdev_irq_status_query irq_status;
@@ -546,7 +535,7 @@ out:
 
 err:
 	pr_err("irq%d: Failed to set port to irq mapping (%d)\n", irq, rc);
-	xen_evtchn_close(evtchn, NR_CPUS);
+	xen_evtchn_close(evtchn, 0);
 	return 0;
 }
 
diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 6df8aac..149e1e9 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -40,6 +40,7 @@
 #include <linux/smp.h>
 #include <linux/percpu.h>
 #include <linux/cpu.h>
+#include <linux/slab.h>
 
 #include <asm/sync_bitops.h>
 #include <asm/xen/hypercall.h>
@@ -385,24 +386,51 @@ static void evtchn_fifo_resume(void)
 	event_array_pages = 0;
 }
 
+struct close_work {
+	struct work_struct work;
+	unsigned int port;
+};
+
+static void evtchn_fifo_close_work(struct work_struct *work)
+{
+	struct close_work *cw = container_of(work, struct close_work, work);
+	struct evtchn_close close;
+
+	while (evtchn_fifo_is_linked(cw->port))
+		cpu_relax();
+
+	close.port = cw->port;
+	if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
+		BUG();
+
+	kfree(cw);
+}
+
 static void evtchn_fifo_close(unsigned port, unsigned int cpu)
 {
-	if (cpu == NR_CPUS)
-		return;
+	struct close_work *cw;
 
-	get_online_cpus();
-	if (cpu_online(cpu)) {
-		if (WARN_ON(irqs_disabled()))
-			goto out;
+	/*
+	 * A port cannot be closed until the LINKED bit is clear.
+	 *
+	 * Reusing an already linked event may: a) cause the new event
+	 * to be raised on the wrong VCPU; or b) cause the event to be
+	 * lost (if the old VCPU is offline).
+	 *
+	 * If the VCPU is offline, its queues must be drained before
+	 * spinning for LINKED to be clear.
+	 */
 
-		while (evtchn_fifo_is_linked(port))
-			cpu_relax();
-	} else {
+	if (!cpu_online(cpu))
 		__evtchn_fifo_handle_events(cpu, true);
-	}
 
-out:
-	put_online_cpus();
+	cw = kzalloc(sizeof(*cw), GFP_ATOMIC);
+	if (!cw)
+		return;
+	INIT_WORK(&cw->work, evtchn_fifo_close_work);
+	cw->port = port;
+
+	schedule_work_on(cpu, &cw->work);
 }
 
 static const struct evtchn_ops evtchn_ops_fifo = {
diff --git a/drivers/xen/events/events_internal.h b/drivers/xen/events/events_internal.h
index d18e123..017cc22 100644
--- a/drivers/xen/events/events_internal.h
+++ b/drivers/xen/events/events_internal.h
@@ -146,10 +146,9 @@ static inline void xen_evtchn_resume(void)
 		evtchn_ops->resume();
 }
 
-static inline void xen_evtchn_op_close(unsigned port, unsigned cpu)
+static inline void xen_evtchn_close(unsigned port, unsigned cpu)
 {
-	if (evtchn_ops->close)
-		return evtchn_ops->close(port, cpu);
+	evtchn_ops->close(port, cpu);
 }
 
 void xen_evtchn_2l_init(void);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread