Date: Fri, 23 Apr 2004 13:57:23 -0400 From: John Baldwin <jhb@FreeBSD.org> To: Daniel Eischen <eischen@vigrid.com> Cc: davidxu@FreeBSD.org Subject: Re: kse_release and kse_wakeup problem (fwd) Message-ID: <200404231357.23096.jhb@FreeBSD.org> In-Reply-To: <Pine.GSO.4.10.10404221759560.15790-100000@pcnet5.pcnet.com> References: <Pine.GSO.4.10.10404221759560.15790-100000@pcnet5.pcnet.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 22 April 2004 06:02 pm, Daniel Eischen wrote: > Sorry, I should have included threads@. > > 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@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200404231357.23096.jhb>