9front - general discussion about 9front
 help / color / mirror / Atom feed
From: qwx@sciops.net
To: 9front@9front.org
Subject: Re: [9front] fix race condition on writes to #v/vgactl
Date: Wed, 17 Aug 2022 14:10:39 +0200	[thread overview]
Message-ID: <E6BE7674A70057AE1801CCEA6209D436@wopr.sciops.net> (raw)
In-Reply-To: <Yvwh8PE+laVJ4v0s@alice>

On Wed Aug 17 01:02:08 +0200 2022, ality@pbrane.org wrote:
> qwx@sciops.net once said:
> > Here's the final patch, mostly the same.  I'll commit this later
> > unless there are any objections.
> 
> There's only two callers of resetscreenimage, two callers of
> vgascreenwin, and four callers of deletescreenimage.
> 
> I would adjust the call sites to include them within the locked
> sections, remove the locking inside them and get rid of the
> repetitive underscore functions.

Makes sense, here's a revised patch.

Thanks!
qwx


diff d86a7ed412555192e2000a9a34b3372f380ec3d0 uncommitted
--- a//sys/src/9/pc/devvga.c
+++ b//sys/src/9/pc/devvga.c
@@ -280,8 +280,10 @@
 			error("bad channel");
 		if(chantodepth(chan) != z)
 			error("depth, channel do not match");
+		qlock(&drawlock);
 		deletescreenimage();
 		setscreensize(scr, r.max.x, r.max.y, z, chan, scr->tilt);
+		qunlock(&drawlock);
 		return;
 
 	case CMactualsize:
@@ -296,6 +298,7 @@
 			error(Ebadarg);
 		if(r.max.x > scr->width || r.max.y > scr->height)
 			error("physical screen bigger than virtual");
+		qlock(&drawlock);
 		deletescreenimage();
 		setactualsize(scr, r);
 		goto Resized;
@@ -324,6 +327,7 @@
 		}
 		if(scr->gscreen == nil)
 			return;
+		qlock(&drawlock);
 		r = actualscreensize(scr);
 		chan = scr->gscreen->chan;
 		z = scr->gscreen->depth;
@@ -332,8 +336,11 @@
 		setactualsize(scr, r);
 		/* no break */
 	case CMdrawinit:
-		if(scr->gscreen == nil)
+		canqlock(&drawlock);
+		if(scr->gscreen == nil){
+			qunlock(&drawlock);
 			error(Enoscreen);
+		}
 		if(scr->dev && scr->dev->drawinit)
 			scr->dev->drawinit(scr);
 		hwblank = scr->blank != nil;
@@ -341,6 +348,7 @@
 	Resized:
 		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/vga.c
+++ b//sys/src/9/pc/vga.c
@@ -183,8 +183,6 @@
 	Point p;
 	int h;
 
-	qlock(&drawlock);
-	
 	h = scr->memdefont->height;
 	r = scr->gscreen->r;
 
@@ -215,11 +213,7 @@
 	curpos = window.min;
 
 	flushmemscreen(r);
-
-	qunlock(&drawlock);
-
 	vgascreenputs(kmesg.buf, kmesg.n);
-
 	screenputs = vgascreenputs;
 }
 
--- a//sys/src/9/port/devdraw.c
+++ b//sys/src/9/port/devdraw.c
@@ -949,7 +949,6 @@
 void
 deletescreenimage(void)
 {
-	dlock();
 	if(screenimage){
 		/* will be freed via screendimage; disable */
 		screenimage->clipr = ZR;
@@ -959,15 +958,12 @@
 		drawfreedimage(screendimage);
 		screendimage = nil;
 	}
-	dunlock();
 }
 
 void
 resetscreenimage(void)
 {
-	dlock();
 	initscreenimage();
-	dunlock();
 }
 
 static Chan*
--- a//sys/src/9/zynq/screen.c
+++ b//sys/src/9/zynq/screen.c
@@ -299,8 +299,8 @@
 		if(chantodepth(chan) != z)
 			error("depth, channel do not match");
 
-		deletescreenimage();
 		eqlock(&drawlock);
+		deletescreenimage();
 		if(memimageinit() < 0){
 			qunlock(&drawlock);
 			error("memimageinit failed");
@@ -310,13 +310,14 @@
 			gscreen = nil;
 		}
 		gscreen = allocmemimage(Rect(0,0,x,y), chan);
-		qunlock(&drawlock);
 		/* wet floor */
 
 	case CMinit:
 		if(gscreen == nil)
 			error("no framebuffer");
+		canqlock(&drawlock);
 		resetscreenimage();
+		qunlock(&drawlock);
 		break;
 	}
 	free(cb);

  reply	other threads:[~2022-08-17 12:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-16  6:12 qwx
2022-08-16 20:44 ` qwx
2022-08-16 20:51   ` Jacob Moody
2022-08-16 23:02   ` Anthony Martin
2022-08-17 12:10     ` qwx [this message]
2022-08-17 12:17       ` ori
2022-08-17 20:21         ` qwx
2022-08-18  2:23           ` ori
2022-08-18  5:36             ` qwx

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E6BE7674A70057AE1801CCEA6209D436@wopr.sciops.net \
    --to=qwx@sciops.net \
    --cc=9front@9front.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).