Subject: Locking in xenevt.c
To: None <port-xen@NetBSD.org>
From: Jed Davis <jdev@panix.com>
List: port-xen
Date: 06/06/2007 23:58:20
--=-=-=

It seems to me that xenevt_fpoll needs to take the same spl and lock
as xenevt_donotify and xenevt_fread to avoid races.  In particular, it
seems that the event handler could be called after the check for an
empty ring but before the corresponding selrecord, thus causing the
calling thread to miss the selnotify() and sleep when it shouldn't.

I'd had a problem with domUs hanging while probing devices, which I
eventually traced down to xenstored not returning from select even
though the xenevt instance it was waiting on had an unread event.  I
could unstick it by doing "call wakeup(selwait)" in ddb a few times,
or by doing other things that use select, like disconnecting and
reconnecting to the domU console.  (This also means that enough
unrelated select()ing in the dom0 could mask the problem.)

The change, which appears to fix this problem for me, seems fairly
obvious, but as I'm not very familiar with the code involved, I'm
posting it here.

-- 
(let ((C call-with-current-continuation)) (apply (lambda (x y) (x y)) (map
((lambda (r) ((C C) (lambda (s) (r (lambda l (apply (s s) l))))))  (lambda
(f) (lambda (l) (if (null? l) C (lambda (k) (display (car l)) ((f (cdr l))
(C k)))))))    '((#\J #\d #\D #\v #\s) (#\e #\space #\a #\i #\newline)))))

--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=xenevt-lock.diff
Content-Description: add spl and simplelock to xenevt_fpoll

Index: sys/arch/xen/xen/xenevt.c
===================================================================
RCS file: /lfs/nb/repo/src/sys/arch/xen/xen/xenevt.c,v
retrieving revision 1.13
diff -u -p -r1.13 xenevt.c
--- sys/arch/xen/xen/xenevt.c	22 Feb 2007 06:48:54 -0000	1.13
+++ sys/arch/xen/xen/xenevt.c	7 Jun 2007 02:48:57 -0000
@@ -560,7 +560,10 @@ xenevt_fpoll(struct file *fp, int events
 {
 	struct xenevt_d *d = fp->f_data;
 	int revents = events & (POLLOUT | POLLWRNORM); /* we can always write */
+	int s;
 
+	s = splsoftxenevt();
+	simple_lock(&d->lock);
 	if (events & (POLLIN | POLLRDNORM)) {
 		if (d->ring_read != d->ring_write) {
 			revents |= events & (POLLIN | POLLRDNORM);
@@ -569,5 +572,7 @@ xenevt_fpoll(struct file *fp, int events
 			selrecord(l, &d->sel);
 		}
 	}
+	simple_unlock(&d->lock);
+	splx(s);
 	return (revents);
 }

--=-=-=--