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 25282 invoked from network); 16 Aug 2022 06:14:35 -0000 Received: from 9front.inri.net (168.235.81.73) by inbox.vuxu.org with ESMTPUTF8; 16 Aug 2022 06:14:35 -0000 Received: from wopr.sciops.net ([216.126.196.60]) by 9front; Tue Aug 16 02:13:00 -0400 2022 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sciops.net; s=20210706; t=1660630315; 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; bh=2XsYuDuJlsT/UoMfXQVIZ64r0Gpim9r41FdXmG+OlF4=; b=tNMW+fLUAgl2X0vfsqPtCPUSjcAkqAhnFydl5/8MLZIAmc+WPPGuxKMrHOMcRrVfNpToXO g5z9Sd95aMJFj8L019nq3mu93gAmq4uuRW3aeLr+6o6o3fmSTIvn4VdzWbZ4x+MB9uXjKR 4TpVsGFe6/wJoGfLt+1xGjqADKavSoI= Received: by wopr.sciops.net (OpenSMTPD) with ESMTPSA id d988abdc (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO) for <9front@9front.org>; Mon, 15 Aug 2022 23:11:54 -0700 (PDT) Message-ID: <559C4D36E88CDC22C329548E7BBFDF3F@wopr.sciops.net> Date: Tue, 16 Aug 2022 08:12:45 +0200 From: qwx@sciops.net To: 9front@9front.org 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: storage core map/reduce realtime-java-scale frontend Subject: [9front] fix race condition on writes to #v/vgactl Reply-To: 9front@9front.org Precedence: bulk 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(); }