From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 17907 invoked from network); 17 Aug 2022 12:13:22 -0000 Received: from 9front.inri.net (168.235.81.73) by inbox.vuxu.org with ESMTPUTF8; 17 Aug 2022 12:13:22 -0000 Received: from wopr.sciops.net ([216.126.196.60]) by 9front; Wed Aug 17 08:10:46 -0400 2022 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sciops.net; s=20210706; t=1660738184; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to; bh=XpEPrzDzYxx8lEfFgoirDBYuFBeNpghhTkQ25F8YFls=; b=lqHy8700/6XjHc8fICzST8vjPOAuQwTgzv1xqRU+83Mk5hNUPpL+NrkpuGE30NLwWi00lX hQFJcwGsO253AVrepkUjuLgFnQBA7AgOGe9dn9wQd7WGqxlThGOOa35OcvaEmJHQmiNuoP Z/ukGtzBhzil7jbMAWUbvA6/E8R+iBk= Received: by wopr.sciops.net (OpenSMTPD) with ESMTPSA id e7a69aa7 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO) for <9front@9front.org>; Wed, 17 Aug 2022 05:09:43 -0700 (PDT) Message-ID: Date: Wed, 17 Aug 2022 14:10:39 +0200 From: qwx@sciops.net To: 9front@9front.org In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: compliant compliant factory module-oriented self-signing DOM locator Subject: Re: [9front] fix race condition on writes to #v/vgactl Reply-To: 9front@9front.org Precedence: bulk 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);