From owner-freebsd-hackers@FreeBSD.ORG Wed Sep 17 13:05:53 2008 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F03DB106564A; Wed, 17 Sep 2008 13:05:53 +0000 (UTC) (envelope-from prvs=114614fff7=marc.loerner@hob.de) Received: from mailgate.hob.de (mailgate.hob.de [212.185.199.3]) by mx1.freebsd.org (Postfix) with ESMTP id 80ABE8FC1D; Wed, 17 Sep 2008 13:05:48 +0000 (UTC) (envelope-from prvs=114614fff7=marc.loerner@hob.de) Received: from imap.hob.de (mail2.hob.de [172.25.1.102]) by mailgate.hob.de (Postfix) with ESMTP id 7A5155200F1; Wed, 17 Sep 2008 14:39:07 +0200 (CEST) Received: from linux03.hob.de (linux03.hob.de [172.22.0.190]) by imap.hob.de (Postfix on SuSE eMail Server 2.0) with ESMTP id 212E0FD3CC; Wed, 17 Sep 2008 14:39:07 +0200 (CEST) From: Marc =?iso-8859-1?q?L=F6rner?= Organization: hob To: John Baldwin Date: Wed, 17 Sep 2008 14:39:37 +0200 User-Agent: KMail/1.6.2 References: <200809041400.04575.marc.loerner@hob.de> <200809101019.30453.marc.loerner@hob.de> <200809101409.49145.jhb@freebsd.org> In-Reply-To: <200809101409.49145.jhb@freebsd.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8bit Message-Id: <200809171439.37151.marc.loerner@hob.de> Cc: freebsd-hackers@freebsd.org Subject: Re: question on asymmetric mtx_[un]lock_sleep X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Sep 2008 13:05:54 -0000 On Wednesday 10 September 2008 20:09, John Baldwin wrote: > On Wednesday 10 September 2008 04:19:30 am Marc Lörner wrote: > > On Tuesday 09 September 2008 21:38, John Baldwin wrote: > > > On Thursday 04 September 2008 08:00:04 am Marc Lörner wrote: > > > > Hello, > > > > I just read through the code of mutexes and turnstiles > > > > and it seems to me that _mtx_lock_sleep and _mtx_unlock_sleep > > > > are some kind of asymmetric when turning SMP and adaptive mutexes > > > > on in kernel-configuration. > > > > > > > > On locking the mutex, we try to "quick" obtain the lock. > > > > If we can't do this, we look, whether some other thread, that's > > > > running, holds the lock and spin until either lock is freed or thread > > > > is not running anymore. In that case we try again to obtain the lock > > > > "quick". If the thread only stopped running but still holds the lock, > > > > we use > > > > > > turnstiles > > > > > > > to wake us up, when the thread unlocks the mutex. > > > > => That seems to be fine and quite symmetric with _mtx_unlock_sleep!! > > > > > > > > But if we're spinning and the other thread gave the mutex free, > > > > we quick-lock the mutex and don't set up a turnstile. > > > > > > > > Now on mtx_unlock_sleep: > > > > - in FreeBSD6/until revision 1.200 turnstiles were tested on > > > > existence. => if turnstile_lookup return NULL we only released the > > > > lock quick. > > > > > > > > - But now, it's never tested if turnstile exists instead we > > > > broadcast/wakeup all threads pending on the turnstile. If this > > > > turnstile is NULL => we > > > > > > access > > > > > > > wrong memory. > > > > > > > > Now my question is: Why can we be sure (in new source) that > > > > turnstile_lookup always returns a valid pointer to an turnstile and > > > > can use returned pointer to call turnstile_broadcast? Am I missing > > something? > > > > > Because it seems that following scenario may occur: > > > > - on locking same scenario as above (=> thread1 now holds the lock) > > > > - thread1 is put off the runqueue > > > > - thread2 now tries to quick unlock mutex and sees that thread1 holds > > > > it => call to mtx_unlock_sleep > > > > - now we try to use turnstile-mechanism and call turnstile_lookup => > > > > returns NULL because no thread waits for wakeup => we call > > > > turnstile_broadcast and crash. > > > > > > Newer locks don't set the CONTESTED flag unless they are actually going > > > to go to sleep. If they succeed in setting CONTESTED or it is already > > > set when they test for it, then they will block on the turnstile. The > > > turnstile chain lock will prevent a concurrent unlock from grabbing the > > > turnstile until the blocking thread is fully asleep. > > > > I see the setting of CONTESTED in case of sleeping in mtx_lock_sleep! > > But there is still the possibility that mtx_lock_sleep "just" obtains the > > lock > > > and doesn't set contested-bit! See described case above (we reach first > > continue in mtx_lock_sleep and test on obtain_lock ends while-loop). > > In that case the lock won't have a turnstile, so mtx_unlock_sleep() will > never be called. > > > Why is this bit not tested in mtx_unlock_sleep? > > If the bit is clear the first attempt to unlock the mutex will succeed, and > mtx_unlock_sleep() won't be called. > > > I think, it's still possible that turnstile_lookup returns NULL. > > And we still have "MPASS(ts != NULL);" in mtx_unlock_sleep that is not > > turned on for GENERIC-kernel (no INVARIANTS support). > > It won't return NULL. > > > And I'm still wondering why the former test on "ts != NULL" went away? > > As I mentioned, previously when a thread used to do an adaptive spin, it > would set the CONTESTED flag before spinning. This could result in the > case that a mutex would have CONTESTED set, but not have an associated > turnstile. Now that the adaptive spinning happens earlier before setting > CONTESTED, mutexes can no longer get into that state. That is, if > CONTESTED is set, the mutex always has an assigned turnstile. If CONTESTED > isn't set, then the first attempt to unlock a mutex will succeed, and > mtx_unlock_sleep() won't be called at all. I got an Page-Not-Present fault on my itanium machine and thought this error is coming from the mutex and turnstile functions. Because when checking on NULL in mtx_unlock_sleep the fault went away. But I was wrong. A little more research resulted in the error lying in the architecture-dependent part of the pcpu.h-macros. So after a thread switched cores he may use the old address of pcpu and curthread that now doesn't hold the right tid anymore. Thanks for your explanations and sorry for the noise! Marc Loerner