kse_release and kse_wakeup problem (fwd)

[Daniel Eischen]

On Mon, 26 Apr 2004, Daniel Eischen wrote:

> On Mon, 26 Apr 2004, David Xu wrote:
> 
> > John Baldwin wrote:
> > > 
> > > 
> > > I.e. do the upcall check in sleepq_catch_signals() right where you already do 
> > > thread_suspend_check(1).  The only reason you have to do this, btw, is 
> > > because the kse_release() code is trying to mess with thread state internals 
> > > using sleepq_abort(), etc.  The other in-kernel code that does that (signals) 
> > > already does the check in sleepq_catch_signals() and has done the same type 
> > > of check in msleep()/tsleep() for quite a while.
> > > 
> > > If the kse_release() stuff was just using sleep/wakeup() rather than trying to 
> > > manually abort sleeps it wouldn't have to be so intimate with the sleep 
> > > interface.
> > > 
> > > Note that thr's thr_wakeup() and thr_sleep() manage to simulate 
> > > synchronization w/o having to abort sleeps, but it is probably also easier to 
> > > do that than for the M:N case.
> > > 
> > 
> > I think libthr will encounters same problem as libpthread with new sleep
> > queue code, because mtx is released too early in msleep before thread
> > markes itself as ON_SLEEPQ, thr_suspend and thr_wakeup have same race 
> > window as kse_release and kse_wakeup. Any code wants to put synchronous
> > bit in td_flags like these codes will be broken.
> 
> I'm experimenting with adding an wakeup_thread() to kern_thread.c
> (to complement wakeup() and wakeup_one()).  If we shouldn't be
> using sleepq's directly, the thread code either needs to
> 
>   a) queue msleep()'ing upcalls/threads itself having them
>      all block on on their own unique wchan's; or
> 
>   b) use a wakeup_thread() that wakes up a specific thread.

Sorry, patch for b) is at:

	http://people.freebsd.org/~deischen/sys.diffs

I don't like using upcall flags, though.  It seems to require taking
the scheduler lock to fiddle with them and that adds some overhead.
Perhaps add another field so we only need to use the proc lock.

There was also a bug in kse_release() where wakeup_one() is called
with the sched_lock held (fixed in above patch).

-- 
Dan Eischen