[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170705225306.2162e0c8@inria.fr>
Date: Wed, 5 Jul 2017 22:53:06 +0200
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: move to a proper __lock_t type
Hello,
On Wed, 5 Jul 2017 12:07:44 -0400 Rich Felker <dalias@...c.org> wrote:
> On Wed, Jul 05, 2017 at 08:30:14AM +0200, Jens Gustedt wrote:
> > So what would you think of the following migration path:
> >
> > (1) Finish the volatile/atomic cleanup by adding a_load, a_load_l
> > and perhaps corresponding store primitives.
>
> I don't think there are any non-atomic stores to atomic objects; if so
> they're either mistakes or they happen synchronized before any
> concurrent access, so that the way the store happens does not matter.
>
> Regarding a_load, I see it as a change unrelated to using a lock type;
> as long as the object is volatile, direct loads still work fine.
>
> Regarding a_load_l, I'd probably like to avoid it. I'm aiming to phase
> out the long/64-bit atomic primitives -- the 64-bit ones are misnomers
> as written, even, and there have always been sketchy aliasing issues
> going on with their implementations, which I'm not sure we ever fixed.
> Right now there are only a few users of them, and if we don't need to
> memcpy the signal mask one, there's no reason it needs to even use the
> same type/representation as a normal sigset_t array. There's actually
> a larger goal to get rid of as much atomic use as possible, outside of
> core sync primitive implementations. Many places they're used as
> dubious speed or size optimizations where a lock would work perfectly
> fine, and would be obviously correct rather than requiring difficult
> correctness analysis. The only places that should really be using
> atomics outside of implementing sync primitives are places where
> there's an AS-safety need that could not be met using locks except
> with a huge hammer (sigprocmask around critical section).
I am not much a fan of the a_..._l primitives either. I came up with
it here, because we would need it in the transformation of the memcpy
logic that we discussed, where the atomic type to be copied is
implemented as "volatile unsigned long[something]".
> > (2) Replace all occurences of "volatile int[2]" by "__lock_t" that
> > encapsulates just that in a struct.
> >
> > (3) Replace the __lock/__unlock pair by the new algorithm
> >
> > (4) Reduce "__lock_t" to a struct that simply has a "volatile int"
> > as sole member.
>
> Can we avoid cosmetics/refactoring like this until the new algorithm
> goes in?
what is "this" here, (2) and (4) ?
> I just have a backlog of stuff to review and commit, and the
> new algorithm is something I really want to get included for release
> since it fixes a visible bug (even if hard to hit the race), and
> having to go back and start again with refactored versions sets that
> back.
Ok, no problem that can wait. Do you need more input for me on the new
algorithm?
> > In all of that, would we need the "__" in the name of the
> > structure? I think this name will never be exported.
>
> I don't think so, but it might be nice to signify to someone reading
> the code that it's not a public type. It might also be mildly nicer in
> the debugger if the application also happens to define a type named
> lock_t.
right
> I'm open to other names too.
I personally don't like the "..._t" naming convention too much, but
"__lock" is already taken for the function. "_Lock", "_Lck" ?
Thanks
Jens
--
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
Content of type "application/pgp-signature" skipped
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.
