This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: thoughts on libatomic


On 04/16/12 11:15, Torvald Riegel wrote:
> Richard, Andrew,
> 
> I had a look at libatomic yesterday, focusing primarily on
> synchronization issues in it.  Here are some comments.  And I think
> there is a bug in it too. Notes are in no particular order.  Let me know
> what you think.
> 
> - seq_cst fences are always used whenever stuff is not relaxed.  I
>   suppose this is a potential future TODO? (default host_config.c)

Fixed.  I think this only really affected powerpc.

> - for acq/rel memorders, we don't need seq_cst fences whenever the
>   atomicity is implemented with a lock
>   (hostconfig.c:pre_barrier/post_barrier)

Err.. where?  This also seems to conflict with...

> - protect_start|end:
>   - BUG: those are called without additional post|pre_barrier calls in
>     cas_n.c, exch_n.c, fop_n.c, load_n.c, store_n.c
>     The lock membar guarantees are too weak if memorder for the atomic
>     access is seq_cst, and if you use more than one lock.
>     (assumes that lock implementation uses typical acq/rel barriers).

... this.  I don't see what multiple locks has to do with seq vs acqrel.

>   - only ptr, no size used, and so atomic types must always be accessed
>     by  same-sized ops.  This does make sense of course, but should we
>     document this explicitly somewhere?

Possibly.  It's also a side-effect of using a WATCH_SIZE that is at least
as large as the largest atomic type supported.

> - load_n.c:
>   - I'm concerned about the CAS on read-only mprotected pages?
>     Why again do we think this is safe?  Does the standard explicitly
>    allow this?  Or should we just use a lock in this case?

Andrew, you had a bit of back-and-forth with someone about this.
Can you dig that up?

> - cas_n.c:
>   - I suppose this is a strong cmpxchg?

Of course.  Weak is restricted to the builtin interface; the library does
not support it.

> - config/posix/lock.c:
>   - I'm not sure that WATCH_SIZE should almost certainly be the same as
>     the cacheline size.  Why?  Which assumptions about the program?

See above.  Plus I assumed that if we have to take a lock, that's got to
dominate over any cache ping-pong that would be played.

>   - page_size optzn: AFAICT, according to the standard (29.4.3), only
>     lock-free atomics are supposed to also be address-free.  So
>     libatomic wouldn't need to be constrained by the page size, or we
>     can use a different hash function too to try to avoid false
>     sharing / conflicts.

Hmm.  Ok, I'll think about this.

>   - Do locks need to care about priority inversion?

At the moment I'm leaving this with the pthread implementation.

> - other notes:
>   - might be better to prefer locks over a kernel cmpxchg perhaps? any
>     archs/platforms that have a lock

Almost certainly not.  When kernel cmpxchg is implemented, that's also
how locks get implemented in libc.  No point using a lock to avoid the
kernel in those cases.

I've updated the tree at

  git://repo.or.cz/gcc/rth.git rth/libatomic



r~


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]