Date: Wed, 10 Sep 2008 10:19:30 +0200 From: Marc =?iso-8859-1?q?L=F6rner?= <marc.loerner@hob.de> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: question on asymmetric mtx_[un]lock_sleep Message-ID: <200809101019.30453.marc.loerner@hob.de> In-Reply-To: <200809091538.08716.jhb@freebsd.org> References: <200809041400.04575.marc.loerner@hob.de> <200809091538.08716.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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). Why is this bit not tested in mtx_unlock_sleep? 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). And I'm still wondering why the former test on "ts != NULL" went away? Regards, Marc Loerner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200809101019.30453.marc.loerner>