From owner-freebsd-current Mon Aug 7 7:17:33 2000 Delivered-To: freebsd-current@freebsd.org Received: from smople.thehub.com.au (smople.thehub.com.au [203.143.240.10]) by hub.freebsd.org (Postfix) with ESMTP id 96A5037B5AF; Mon, 7 Aug 2000 07:17:23 -0700 (PDT) (envelope-from mckay@thehub.com.au) Received: from gift.home (ppp215.dyn248.pacific.net.au [203.143.248.215]) by smople.thehub.com.au (8.9.3/8.9.1) with ESMTP id AAA92482; Tue, 8 Aug 2000 00:17:08 +1000 (EST) Received: (from mckay@localhost) by gift.home (8.9.3/8.9.3) id AAA00524; Tue, 8 Aug 2000 00:17:48 +1000 (EST) (envelope-from mckay) Date: Tue, 8 Aug 2000 00:17:48 +1000 (EST) From: Stephen McKay Message-Id: <200008071417.AAA00524@gift.home> To: Alfred Perlstein Cc: Mike Smith , Stephen McKay , freebsd-current@FreeBSD.ORG, dillon@FreeBSD.ORG Subject: Re: Ugly, slow shutdown References: <20000807011854.Q4854@fw.wintelcom.net> <200008070836.BAA02380@mass.osd.bsdi.com> <20000807015018.R4854@fw.wintelcom.net> Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG > * Mike Smith [000807 01:25] wrote: > > > * Stephen McKay [000805 08:49] wrote: > > > > > > > > ... every sleeping process should expect > > > > to be woken for no reason at all. Basic kernel premise. > > > > > > You better bet it's controversial, this isn't "Basic kernel premise" > > > > Actually, that depends. It is definitely poor programming practice to > > not check the condition for which you slept on wakeup. > > Stephen's patches didn't give them that option, the syncer could be > in some other part of vfs that doesn't expect to be woken up, perhaps > in uniterruptable sleep... perhaps waiting for a DMA transfer? > > How does one check if the data filled into a buffer is actually from > the driver and not just stale? The time honoured standard is: raise cpu priority while (we do not have exclusive use of some item) { set some sort of "I want this item" flag (optional) sleep on a variable related to the item } use the item/data we waited for lower cpu priority A typical example from vfs_subr.c: s = splbio(); while (vp->v_numoutput) { vp->v_flag |= VBWAIT; error = tsleep((caddr_t)&vp->v_numoutput, slpflag | (PRIBIO + 1), "vinvlbuf", slptimeo); if (error) { splx(s); return (error); } } ... the code plays a little with vp here ... splx(s); A simpler example from swap_pager.c: s = splbio(); while ((bp->b_flags & B_DONE) == 0) { tsleep(bp, PVM, "swwrt", 0); } ... code uses bp here ... splx(s); Both of these examples are safe from side effects due to waking up early. This is how all such code should be. To do otherwise is to introduce possible race conditions. At your prompting, though, I've looked at more code and have found an example that violates this principle. I assume it is a bug waiting to bite us. In the 4.1.0 source (sorry, that's all I have on operational computers at this moment) line 581 of vfs_bio.c sleeps without looping. It would seem that Alfred's assertion of lurking danger is correct. This stuff should be fixed. > > > *boom* *crash* *ow* :) > > > > Doctor: So don't do that. > > > > In this case, the relevant processes just need to learn to check whether > > they've been woken in order to die. > > No, they need to signify that it's safe to wake them up early. When I return to the land of FreeBSD I'll offer a speedup that does not wake processes in arbitrary places (to avoid tickling lurking bugs). To do this I would make processes that want to use the suspension mechanism call a routine in kern_kthread.c for their just-loafing-about sleep. Then that module will have enough information to do the job quickly. And back to the simpler bit (the bike shed bit). Does everyone else actually *like* the verbose messages currently used? And the gratuitous extra newline in the "syncing..." message? Stephen. PS My main machine has blown its power supply. Contact with me will be patchy. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message