From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 4 Aug 2008 12:18:53 -0400
Subject: [xen] event channel lock and barrier
Message-id: m34p603g9e.fsf@crossbow.pond.sub.org
O-Subject: [PATCH RHEL-5.3] Event channel lock and barrier
Bugzilla: 457086
RH-Acked-by: Chris Lalancette <clalance@redhat.com>
RH-Acked-by: Don Dutile <ddutile@redhat.com>

The evtchn driver is the kernel part of Xen's event channel
communication mechanism.

The version in RHEL-5 lacks locks and memory barriers.  This has led
to xenstore hangs on IA-64, but there's really no telling what else
the bug could break under load.

Fix backported from upstream, see patch for details.

I asked reporter (Fujitsu) to test the fix on IA-64.  I'll follow up
with results.

Bug 457086

Please ACK.

Trivially derived from

# HG changeset patch
# User kfraser@localhost.localdomain
# Date 1175086323 -3600
# Node ID 77b210daefee957faa220e94501ace6bf50e3b97
# Parent  7e6ef2b914aa739c51f15b0700aa54de41b4707d
linux evtchn: Read function in evtchn driver requires locking.
Signed-off-by: Keir Fraser <keir@xensource.com>

# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1216724382 -3600
# Node ID 8a3dc4fdb4785447398e983c22241c38f128663b
# Parent  905f275ed4d8a7bca99dba273e9bc838f605e8b9
linux/evtchn: Add memory barriers to evtchn ring accesses.

Xenstore infrequently hangs up on IA64.
Actually the xenstored is still alive but no response from
xenstore-XXX commands.

After tracking down, I've found that evtchn_read() infrequently
returns a wrong evtchn port number and evtchn_write() never
unmask the exact port.

Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>

Yes, updates of the ring_prod and ring_cons are separately protected
by different locks/mutexes, but the data communication between
producer and consumer is lock-free. Barriers are needed.

Acked-by: Keir Fraser <keir.fraser@citrix.com>

diff --git a/drivers/xen/evtchn/evtchn.c b/drivers/xen/evtchn/evtchn.c
index 4323b58..d9b95c3 100644
--- a/drivers/xen/evtchn/evtchn.c
+++ b/drivers/xen/evtchn/evtchn.c
@@ -47,6 +47,7 @@
 #include <linux/irq.h>
 #include <linux/init.h>
 #include <linux/gfp.h>
+#include <linux/mutex.h>
 #include <xen/evtchn.h>
 #include <xen/public/evtchn.h>
 
@@ -56,6 +57,7 @@ struct per_user_data {
 #define EVTCHN_RING_MASK(_i) ((_i)&(EVTCHN_RING_SIZE-1))
 	evtchn_port_t *ring;
 	unsigned int ring_cons, ring_prod, ring_overflow;
+	struct mutex ring_cons_mutex; /* protect against concurrent readers */
 
 	/* Processes wait on this queue when ring is empty. */
 	wait_queue_head_t evtchn_wait;
@@ -78,6 +80,7 @@ void evtchn_device_upcall(int port)
 	if ((u = port_user[port]) != NULL) {
 		if ((u->ring_prod - u->ring_cons) < EVTCHN_RING_SIZE) {
 			u->ring[EVTCHN_RING_MASK(u->ring_prod)] = port;
+			wmb(); /* Ensure ring contents visible */
 			if (u->ring_cons == u->ring_prod++) {
 				wake_up_interruptible(&u->evtchn_wait);
 				kill_fasync(&u->evtchn_async_queue,
@@ -108,12 +111,17 @@ static ssize_t evtchn_read(struct file *file, char __user *buf,
 		count = PAGE_SIZE;
 
 	for (;;) {
+		mutex_lock(&u->ring_cons_mutex);
+
+		rc = -EFBIG;
 		if (u->ring_overflow)
-			return -EFBIG;
+			goto unlock_out;
 
 		if ((c = u->ring_cons) != (p = u->ring_prod))
 			break;
 
+		mutex_unlock(&u->ring_cons_mutex);
+
 		if (file->f_flags & O_NONBLOCK)
 			return -EAGAIN;
 
@@ -141,20 +149,25 @@ static ssize_t evtchn_read(struct file *file, char __user *buf,
 		bytes2 = count - bytes1;
 	}
 
+	rc = -EFAULT;
+	rmb(); /* Ensure that we see the port before we copy it. */
 	if (copy_to_user(buf, &u->ring[EVTCHN_RING_MASK(c)], bytes1) ||
 	    ((bytes2 != 0) &&
 	     copy_to_user(&buf[bytes1], &u->ring[0], bytes2)))
-		return -EFAULT;
+		goto unlock_out;
 
 	u->ring_cons += (bytes1 + bytes2) / sizeof(evtchn_port_t);
+	rc = bytes1 + bytes2;
 
-	return bytes1 + bytes2;
+ unlock_out:
+	mutex_unlock(&u->ring_cons_mutex);
+	return rc;
 }
 
 static ssize_t evtchn_write(struct file *file, const char __user *buf,
 			    size_t count, loff_t *ppos)
 {
-	int  rc, i;
+	int rc, i;
 	evtchn_port_t *kbuf = (evtchn_port_t *)__get_free_page(GFP_KERNEL);
 	struct per_user_data *u = file->private_data;
 
@@ -164,18 +177,16 @@ static ssize_t evtchn_write(struct file *file, const char __user *buf,
 	/* Whole number of ports. */
 	count &= ~(sizeof(evtchn_port_t)-1);
 
-	if (count == 0) {
-		rc = 0;
+	rc = 0;
+	if (count == 0)
 		goto out;
-	}
 
 	if (count > PAGE_SIZE)
 		count = PAGE_SIZE;
 
-	if (copy_from_user(kbuf, buf, count) != 0) {
-		rc = -EFAULT;
+	rc = -EFAULT;
+	if (copy_from_user(kbuf, buf, count) != 0)
 		goto out;
-	}
 
 	spin_lock_irq(&port_user_lock);
 	for (i = 0; i < (count/sizeof(evtchn_port_t)); i++)
@@ -321,9 +332,11 @@ static int evtchn_ioctl(struct inode *inode, struct file *file,
 
 	case IOCTL_EVTCHN_RESET: {
 		/* Initialise the ring to empty. Clear errors. */
+		mutex_lock(&u->ring_cons_mutex);
 		spin_lock_irq(&port_user_lock);
 		u->ring_cons = u->ring_prod = u->ring_overflow = 0;
 		spin_unlock_irq(&port_user_lock);
+		mutex_unlock(&u->ring_cons_mutex);
 		rc = 0;
 		break;
 	}
@@ -371,6 +384,8 @@ static int evtchn_open(struct inode *inode, struct file *filp)
 		return -ENOMEM;
 	}
 
+	mutex_init(&u->ring_cons_mutex);
+
 	filp->private_data = u;
 
 	return 0;