Subject: Re: Bare tsleep in dev/ic/wdc.c
To: Manuel Bouyer <bouyer@antioche.lip6.fr>
From: Constantine Sapuntzakis <csapuntz@stanford.edu>
List: tech-kern
Date: 07/22/1999 20:27:40
Manuel Bouyer <bouyer@antioche.lip6.fr> writes:

> On Sat, Jul 17, 1999 at 12:42:35PM -0700, Constantine Sapuntzakis wrote:
> > 
> > >From wdc.c, around line 1021:
> > 
> > 		if (wdc_c->flags & AT_WAIT) {
> > 			tsleep(wdc_c, PRIBIO, "wdccmd", 0);
> > 			ret = WDC_COMPLETE;
> > 		} else {
> > 
> > 
> > Shouldn't that be:
> > 
> > 		if (wdc_c->flags & AT_WAIT) {
> > 			while (!(wdc_c->flags & AT_DONE)) {
> > 				tsleep(wdc_c, PRIBIO, "wdccmd", 0); 
> > 			}
> > 			ret = WDC_COMPLETE;
> > 		} else {
> > 	
> > to prevent spurious wakeups from causing premature returns? I ran into
> > this problem on boot-up.
> 
> How can a spurious wakeup happen ? The only place where wakeup() is called
> on wdc_c is in __wdccommand_done(), and AT_DONE is set before ...
> 
> I'm certainly missing something but I'd like to know what :)
> 

Well, how about if somebody calls setrunnable() on the sleeping
process. That'll wake your process from the tsleep. Yes, I know
they're not supposed to do that, but shit happens.

setrunnable can be called from anywhere in the kernel. The IDE driver
could be the first to notice that setrunnable shouldn't have been called
- by hanging. Or it can deal with it correctly by checking
AT_DONE. 

If you don't want a put a loop there, I would recommend at least a DIAGNOSTIC
check to make sure AT_DONE is set after tsleep returns and panic otherwise.

-Costa