On Tue, May 29, 2007 at 09:02:07AM +0200, Reinoud Zandijk wrote:

> On Sat, Apr 28, 2007 at 04:37:31PM +0100, Andrew Doran wrote:
> > > > That code is simply bogus; it needs to look more like this:
> > > >
> > > > s = splfoo()
> > > >   while (! (sc->flags & SCF_INTERRUPTED)) {
> > > >         tsleep();
> > > >   }
> > > > splx(s);
> 
> yes, i think the author meant:
> 
> s = splfoo();
> while (!(sc->flags & SCF_INTERURUPTED)) {
> 	splx(s);

<- there is a race here. Interrupts are no longer blocked, an interrupt can
fire and the interrupt handler can set SCF_INTERRUPTED. It then issues a
wakeup(), but the thread has not yet completed tsleep(), and so is not
asleep. So the thread misses the wakeup, and sleeps forever instead..

> 	tsleep(...);
> 	s = splfoo();
> }
> splx(s);
> 
> How would otherwise the interrupt handler be able to enter the spl 
> level/interrupt level needed to modify the value's :-) Or am i wrong in 
> this and is tsleep() taking care of that in this situation?

The call to tsleep needs to be made with the interrupt blocked. tsleep
saves and restores the SPL automatically.

> > If you are targeting -current:
> > 
> > 	mutex_enter(&foo_lock);
> > 	while ((sc->flags & SCF_INTERRUPTED) == 0)
> > 		cv_wait(&foo_cv, &foo_lock);
> > 	mutex_exit(&foo_lock);
> > 
> > :-)
> 
> Thats a lot better indeed! A lot cleaner too....

Essentially the same thing, but using locks. cv_wait has no knowledge
of the lock you want to drop, so you have to tell it.

Cheers,
Andrew