Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 22 Dec 2011 13:28:41 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        "Dag-Erling =?utf-8?q?Sm=C3=B8rgrav?=" <des@des.no>
Cc:        Poul-Henning Kamp <phk@phk.freebsd.dk>, freebsd-threads@freebsd.org, freebsd-arch@freebsd.org
Subject:   Re: [Patch] C1X threading support
Message-ID:  <201112221328.41221.jhb@freebsd.org>
In-Reply-To: <86zkeksftq.fsf@ds4.des.no>
References:  <73233.1324389741@critter.freebsd.dk> <201112211028.26780.jhb@freebsd.org> <86zkeksftq.fsf@ds4.des.no>

index | next in thread | previous in thread | raw e-mail

On Thursday, December 22, 2011 11:59:13 am Dag-Erling Smørgrav wrote:
> John Baldwin <jhb@freebsd.org> writes:
> > Dag-Erling Smørgrav <des@des.no> writes:
> > > Poul-Henning Kamp <phk@phk.freebsd.dk> writes:
> > > > 	mtx_unlock(l)
> > > > 	{
> > > > 		assert(l->id == thread_id);
> > > > 		l->id = NULL;
> > > > 		atomic_magic_unlock(l->lock_field)
> > > > 	}
> > > susceptible to race conditions
> > How so?
> 
> I should have specified "if called from a thread that does not own the
> mutex"

You can do the same check as mtx_assert_held() internally and fail the unlock 
request in that case (or abort or what have you).

> > > > 	mtx_assert_held(l)
> > > > 	{
> > > > 		assert(l->lock-field != 0);
> > > > 		assert(l->id == thread_id);
> > > > 	}
> > > susceptible to race conditions
> > How so?
> 
> I was going to point out that the state of the mutex can change between
> the two asserts, but as you say, at least one of them is guaranteed to
> fail...  *if* you assume that these fields can be read atomically, which
> was one of my objections.

I do think these have to be atomic.  I think lock-field must be able to be 
atomically read by definition.  I think it is not an unreasonable requirement 
to have the implementation implement a thread_id that fits in an atomic type 
if it is not able to encode the thread_id into the lock cookie itself.

I do think if l->id was not atomic it might be feasible to see a value while
the thread is not locked that is equivalent to the current thread's ID based
on observing different parts of l->id from different writes.  There might be 
ways the implementation could guard against that however.  In practice though 
I think pointers are atomic with regards to loads and stores on nearly all 
machines.

-- 
John Baldwin


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201112221328.41221.jhb>