Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 27 Aug 2014 23:30:26 +0200
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 2/2] avoid taking _c_lock if we know it isn't
 necessary

Am Mittwoch, den 27.08.2014, 16:07 -0400 schrieb Rich Felker:
> On Wed, Aug 27, 2014 at 11:57:47AM +0200, Jens Gustedt wrote:
> > +		/* Since this leaving waiter might not have held the _c_lock, the following       */
> > +		/* __wake might be issued when the signaler is still inside its CS.               */
> > +		/* But if so, this avoids a __wait of the signaler, which more important.         */
> > +		/* This should not target any spurious wake up in any other thread:               */
> > +		/* ref is on the stack of the signaler, and that signaler is still alive.         */
> >  		if (ref) {
> >  			if (a_fetch_add(ref, -1)==1)
> >  				__wake(ref, 1, 1);
> 
> Can't you avoid that with the design I suggested, having the signaler
> use an extra ref count on itself, which it decrements right before
> waiting?

Probably, but I have not completely thought this through. But if so
this would round up this series nicely with a third patch.

> Aside from that, based on my reading so far, these patches look like
> they should work correctly. But since we both want to get C11 threads
> done,

Exactly, that was my idea, freeze the main ideas in some short patches
and have them in the list archives for future use.

> let's put them aside for now (pending some testing for
> measurable benefits).


> I also have some other potential changes to this
> code based on my latest comments to:
> 
> http://austingroupbugs.net/view.php?id=609
> 
> regarding things they seem to deem as requirements, and which musl
> does not satisfy, that are specified in non-normative text. So there's
> likely to be more cond var work to do before the release still...

Ah, the cancelation stuff. As if condition variables wouldn't be
complicated enough already, without cancelation. We already have two
different ordered sequences of events, those on the cv and those on
the mutex. The discussion (and our implementation struggles) already
shows how difficult it is to get these two linear sequences ordered in
a convenient way. If you add a third set of events that are neither
ordered among themselves (cancelation to different threads are
asynchronous) nor with any of the two sequences, the semantics aren't
clear at all. (This is why I think that generally thread cancelation
is not a good idea, and why it is not very widely used. It contributes
for more than 50% to the complexity of the implementation of
pthreads.)

But with the current implementation, I would think that it basically
fulfills (or can be easily made to fulfill) the requirement that
cancelation would not be "consuming" a signal when some other thread
is available. We are marking threads as WAITING, LEAVING or SIGNALED
and only for WAITING, a thread can be consired "blocked" on the
cv. The transition between these is atomic, and so once a signaler
marked a thread SIGNALED, it is not blocked and has rightly consumed
the signal.

I didn't check, though, if timedwait returns 0 in that case the final
value is SIGNALED. If not, that would probably be a reasonable way to
go. Something like

if (SIGNALED && "mutex sucessfully acquired") return 0
else return the proper error code as before

Jens



-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.