[9front] fix race condition on writes to #v/vgactl

[qwx@sciops.net]

Hello,

I've been experiencing a seemingly impossible kernel panic when
messing with vgactl.  Occasionally, writing multiple commands to
/dev/vgactl rapidly may cause an assert to fail
(/sys/src/9/port/devdraw.c:/assert).  The panic is perfectly
reproducible by sending `softscreen off' commands quickly (tested on
x61t):

	; for(i in `{seq 10}) echo softscreen off >/dev/vgactl

The panic occurs within the first few iterations.  Moving some locks
out of individual functions to hold them for the entire step fixes
this.  The patch below works, but qunlock may be called without the
drawlock being held, I need to recheck and clean up before commiting
anything.  Credit to moody for suggesting looking in this direction!

Thoughts?

Thanks,
qwx


diff 69352f66684cbc75e2cd68c80cd028324964c5aa uncommitted
--- a//sys/src/9/pc/devvga.c
+++ b//sys/src/9/pc/devvga.c
@@ -296,7 +296,8 @@
 			error(Ebadarg);
 		if(r.max.x > scr->width || r.max.y > scr->height)
 			error("physical screen bigger than virtual");
-		deletescreenimage();
+		qlock(&drawlock);
+		_deletescreenimage();
 		setactualsize(scr, r);
 		goto Resized;
 	
@@ -324,10 +325,11 @@
 		}
 		if(scr->gscreen == nil)
 			return;
+		qlock(&drawlock);
 		r = actualscreensize(scr);
 		chan = scr->gscreen->chan;
 		z = scr->gscreen->depth;
-		deletescreenimage();
+		_deletescreenimage();
 		setscreensize(scr, scr->width, scr->height, z, chan, tilt);
 		setactualsize(scr, r);
 		/* no break */
@@ -339,8 +341,9 @@
 		hwblank = scr->blank != nil;
 		hwaccel = scr->fill != nil || scr->scroll != nil;
 	Resized:
-		vgascreenwin(scr);
-		resetscreenimage();
+		_vgascreenwin(scr);
+		_resetscreenimage();
+		qunlock(&drawlock);
 		return;
 
 	case CMlinear:
--- a//sys/src/9/pc/screen.c
+++ b//sys/src/9/pc/screen.c
@@ -64,17 +64,11 @@
 void
 setactualsize(VGAscr *scr, Rectangle r)
 {
-	qlock(&drawlock);
-
 	r.min = ZP;
 	r.max = tiltsize(scr->tilt, r.max);
-	if(rectclip(&r, scr->gscreen->r) == 0){
-		qunlock(&drawlock);
+	if(rectclip(&r, scr->gscreen->r) == 0)
 		return;
-	}
 	scr->gscreen->clipr = r;
-
-	qunlock(&drawlock);
 }
 
 static char*
@@ -136,11 +130,8 @@
 {
 	char *err;
 
-	qlock(&drawlock);
-	if(waserror()){
-		qunlock(&drawlock);
+	if(waserror())
 		nexterror();
-	}
 
 	if(memimageinit() < 0)
 		error("memimageinit failed");
@@ -169,7 +160,6 @@
 		cursoron();
 	}
 
-	qunlock(&drawlock);
 	poperror();
 }
 
--- a//sys/src/9/pc/screen.h
+++ b//sys/src/9/pc/screen.h
@@ -151,6 +151,8 @@
 /* devdraw.c */
 extern void	deletescreenimage(void);
 extern void	resetscreenimage(void);
+extern void	_deletescreenimage(void);
+extern void	_resetscreenimage(void);
 extern void	setscreenimageclipr(Rectangle);
 extern void	drawflush(void);
 extern QLock	drawlock;
@@ -157,6 +159,7 @@
 
 /* vga.c */
 extern void	vgascreenwin(VGAscr*);
+extern void	_vgascreenwin(VGAscr*);
 extern void	vgaimageinit(ulong);
 extern void	vgalinearpci(VGAscr*);
 extern void	vgalinearaddr(VGAscr*, uvlong, vlong);
--- a//sys/src/9/pc/vga.c
+++ b//sys/src/9/pc/vga.c
@@ -176,7 +176,7 @@
 }
 
 void
-vgascreenwin(VGAscr* scr)
+_vgascreenwin(VGAscr* scr)
 {
 	Memimage *i;
 	Rectangle r;
@@ -183,8 +183,6 @@
 	Point p;
 	int h;
 
-	qlock(&drawlock);
-	
 	h = scr->memdefont->height;
 	r = scr->gscreen->r;
 
@@ -215,12 +213,16 @@
 	curpos = window.min;
 
 	flushmemscreen(r);
-
-	qunlock(&drawlock);
-
 	vgascreenputs(kmesg.buf, kmesg.n);
-
 	screenputs = vgascreenputs;
+}
+
+void
+vgascreenwin(VGAscr* scr)
+{
+	qlock(&drawlock);
+	_vgascreenwin(scr);
+	qunlock(&drawlock);
 }
 
 /*
--- a//sys/src/9/port/devdraw.c
+++ b//sys/src/9/port/devdraw.c
@@ -947,9 +947,8 @@
 }
 
 void
-deletescreenimage(void)
+_deletescreenimage(void)
 {
-	dlock();
 	if(screenimage){
 		/* will be freed via screendimage; disable */
 		screenimage->clipr = ZR;
@@ -959,14 +958,27 @@
 		drawfreedimage(screendimage);
 		screendimage = nil;
 	}
+}
+
+void
+deletescreenimage(void)
+{
+	dlock();
+	_deletescreenimage();
 	dunlock();
 }
 
 void
+_resetscreenimage(void)
+{
+	initscreenimage();
+}
+
+void
 resetscreenimage(void)
 {
 	dlock();
-	initscreenimage();
+	_resetscreenimage();
 	dunlock();
 }