kse_release and kse_wakeup problem (fwd)

[John Baldwin]

On Thursday 22 April 2004 06:02 pm, Daniel Eischen wrote:
> Sorry, I should have included threads at .
>
> On Thu, 22 Apr 2004, John Baldwin wrote:
> > On Thursday 22 April 2004 09:01 am, Daniel Eischen wrote:
> > > What do you guys think of this patch?
> >
> > I think that the thread code should check for the upcall case the same
> > way
>
> The thread code where?  Everywhere msleep() is called?

No, in sleepq_catch_signals() just as I quoted below:

> > that we check for signals by calliing cursig() in sleepq_catch_signals(),
> > that is:
> >
> >         /* Mark thread as being in an interruptible sleep. */
> >         mtx_lock_spin(&sched_lock);
> >         MPASS(TD_ON_SLEEPQ(td));
> >         td->td_flags |= TDF_SINTR;
> >         mtx_unlock_spin(&sched_lock);
> >         sleepq_release(wchan);
> >
> >         /* See if there are any pending signals for this thread. */
> >         PROC_LOCK(p);
> >         mtx_lock(&p->p_sigacts->ps_mtx);
> >         sig = cursig(td);
> >         mtx_unlock(&p->p_sigacts->ps_mtx);
> >         if (sig == 0 && thread_suspend_check(1))
> >                 sig = SIGSTOP;
> >         PROC_UNLOCK(p);
> >
> >         /*
> >          * If there were pending signals and this thread is still on
> >          * the sleep queue, remove it from the sleep queue.
> >          */
> >
> > The thread_suspend_check() code should also be checking for the UPCALL
> > case and as well.  Maybe something like:
> >
> > 	if (sig == 0 && thread_suspend_check(1))
> > 		sig == SIGSTOP;
> > 	if (sig == 0 && thread_upcall_check())
> > 		doing_upcall = 1;
> >
> > and then dequeue if we are doing an upcall just like we do if there is
> > already a signal.  This mirrors the way we handle signals since UPCALLs
> > are a kind of special cased signal.  The patch below has incorrect
> > locking (td_flags could get trashed) by the way.

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.

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org