> * Mike Smith <[EMAIL PROTECTED]> [000807 01:25] wrote:
> > > * Stephen McKay <[EMAIL PROTECTED]> [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 [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to