Date: Wed, 10 Sep 2008 14:09:48 -0400 From: John Baldwin <jhb@freebsd.org> To: Marc =?iso-8859-1?q?L=F6rner?= <marc.loerner@hob.de> Cc: freebsd-hackers@freebsd.org Subject: Re: question on asymmetric mtx_[un]lock_sleep Message-ID: <200809101409.49145.jhb@freebsd.org> In-Reply-To: <200809101019.30453.marc.loerner@hob.de> References: <200809041400.04575.marc.loerner@hob.de> <200809091538.08716.jhb@freebsd.org> <200809101019.30453.marc.loerner@hob.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 10 September 2008 04:19:30 am Marc L=F6rner wrote: > On Tuesday 09 September 2008 21:38, John Baldwin wrote: > > On Thursday 04 September 2008 08:00:04 am Marc L=F6rner 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 runni= ng, > > > 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. > > > =3D> 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 existenc= e. > > > =3D> if turnstile_lookup return NULL we only released the lock quic= k. > > > > > > - But now, it's never tested if turnstile exists instead we > > > broadcast/wakeup all threads pending on the turnstile. If this turnst= ile > > > is NULL =3D> 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 c= an > > > use returned pointer to call turnstile_broadcast? Am I missing=20 something? > > > > > > Because it seems that following scenario may occur: > > > - on locking same scenario as above (=3D> thread1 now holds the lock) > > > - thread1 is put off the runqueue > > > - thread2 now tries to quick unlock mutex and sees that thread1 holds= it > > > =3D> call to mtx_unlock_sleep > > > - now we try to use turnstile-mechanism and call turnstile_lookup =3D> > > > returns NULL because no thread waits for wakeup =3D> 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. >=20 > 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= =20 lock=20 > and doesn't set contested-bit! See described case above (we reach first=20 > 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 ne= ver=20 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= =20 mtx_unlock_sleep() won't be called. > I think, it's still possible that turnstile_lookup returns NULL. > And we still have "MPASS(ts !=3D 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 !=3D NULL" went away? As I mentioned, previously when a thread used to do an adaptive spin, it wo= uld=20 set the CONTESTED flag before spinning. This could result in the case that= a=20 mutex would have CONTESTED set, but not have an associated turnstile. Now= =20 that the adaptive spinning happens earlier before setting CONTESTED, mutexe= s=20 can no longer get into that state. That is, if CONTESTED is set, the mutex= =20 always has an assigned turnstile. If CONTESTED isn't set, then the first=20 attempt to unlock a mutex will succeed, and mtx_unlock_sleep() won't be=20 called at all. =2D-=20 John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200809101409.49145.jhb>