ado@elsie.UUCP (Arthur David Olson) (10/07/85)
The 4.2bsd version of the kernel function "bflush" calls "spl6", goes through some gyrations, and eventually may call one of the disk "strategy" functions. One of these in particular, "hpstrategy", calls "spl5". Now it seems to me that theoretically the spl5 call when things are chugging along at priority level 6 is a no-no--although in this case there doesn't seem to be any bad effect in practice. And so some questions to all you wizards: 1. Should "bflush" actually be doing a "spl5" call? 2. Should "hpstrategy" actually be doing a "spl6" call? 3. Should "bflush" be doing a "splx(s)" call (to restore the processor level to its entry value) before calling the disk strategy function? 4. Should spl4, spl5, spl6, and spl7 check to ensure that the priority level is being raised before changing the priority level? -- UNIX is an AT&T Bell Laboratories trademark. Checkers is a Richard Nixon trademark. -- UUCP: ..decvax!seismo!elsie!ado ARPA: elsie!ado@seismo.ARPA DEC, VAX and Elsie are Digital Equipment and Borden trademarks
chris@umcp-cs.UUCP (Chris Torek) (10/08/85)
In this particular case it is safe: the spl6() in bflush() is to protect bp->b_flags, since spl6() blocks disk operations (the things that invoke biodone()), and the flags are actually all stashed away by the time hpstrategy() is called. The spl5() in hpstrategy() is presumably to protect bp->b_cylin for disksort(); I am not convinced that this is (a) required or (b) effective, but as you said it obviously is not breaking anything. `s = spl<k>();' should probably be rewritten as `s = raiseplto<k>();'; it is a rather rare event when the previous priority level is known and/or the new level should be set regardless of the previous level. Of course, the real problem is that raising an interrupt priority level is not, after all, such a marvellous way of guaranteeing exclusion. It is simple, and that is an advantage, but (e.g.) it makes for long interrupt latency in some cases, and does not work well with multiple CPUs. -- In-Real-Life: Chris Torek, Univ of MD Comp Sci Dept (+1 301 454 4251) UUCP: seismo!umcp-cs!chris CSNet: chris@umcp-cs ARPA: chris@mimsy.umd.edu
dpb@laidbak.UUCP (Darryl Baker) (10/08/85)
In article <5237@elsie.UUCP> ado@elsie.UUCP (Arthur David Olson) writes: >The 4.2bsd version of the kernel function "bflush" calls "spl6", >goes through some gyrations, and eventually may call one of the disk "strategy" >functions. One of these in particular, "hpstrategy", calls "spl5". > >Now it seems to me that theoretically the spl5 call when things are chugging >along at priority level 6 is a no-no--although in this case there doesn't seem >to be any bad effect in practice. > >And so some questions to all you wizards: > >1. Should "bflush" actually be doing a "spl5" call? Yes, there is nothing done at clock interrupt time that effects bflush. >2. Should "hpstrategy" actually be doing a "spl6" call? I don't know. >3. Should "bflush" be doing a "splx(s)" call (to restore the processor > level to its entry value) before calling the disk strategy function? No, it can be called at user level (spl0) and interrupt routines will muck with buffers. >4. Should spl4, spl5, spl6, and spl7 check to ensure that the priority > level is being raised before changing the priority level? Maybe, this is a matter of taste. >-- You have some good thoughts but missed the point that the bflush algorithm is terrible especially if you have a great (>256) number of buffers. I will include a suggestion for a different bflush algorithm. Warning I have not yet tested this on a 4.2 BSD System but I have seen something like this used on a System V System. The difference in the two alorithm is that the current one finds one buffer to write for each pass through the freelist until it makes it all the way through and the one I'm suggesting writes all the buffers it can find in the headers for each pass until it makes one complete pass without finding one to write. /* * make sure all write-behind blocks * on dev (or NODEV for all) * are flushed out. * (from umount and update) */ bflush(dev) register dev_t dev; { register struct buf *bp; register int bufferwritten; register int s; s = spl5(); do { bufferwritten = FALSE; for (bp = buf ; bp < &buf[nbuf]; bp++) { spl5(); /* needed because drivers may mung the spl */ /* even though they shouldn't */ if ( (bp->b_flags&B_DELWRI) && (dev == NODEV || dev==bp->b_dev) && ((bp->b_flags&B_BUSY) != B_BUSY)) { bp->b_flags |= B_ASYNC; notavail(bp); /* This macro should also be * modified to do an spl5 not * spl6 */ bufferwritten = TRUE; bwrite(bp); } } } while(bufferwritten); splx(s); } -- from the sleepy terminal of Darryl Baker [ihnp4!]laidbak!dpb