Re: [PATCH] Use C11 atomics instead of atomic_increment(_val)

[Wilco Dijkstra <Wilco.Dijkstra@arm.com>]

Hi Adhemerval,

In general you only need acquire/release for locks that synchronize access to
shared data which does not use atomic accesses. If there is no such shared
data, there is no need for acquire/release. Similarly only using acquire and never
release (as the old atomics did) makes no sense - you need a release atomic
to synchronize with.

> I am not sure if relaxed MO is correct here, shouldn't it synchronize with the
> __nptl_setxid_sighandler ?

The counter cntr in nptl/nptl_setxid.c is just a simple atomic counter to keep
track of the number of outstanding signals that were sent to threads. There is
no data it is trying to synchronize with, so release/acquire semantics do not
make sense here.

> Not sure if this is correct for hurd, __pthread_total is used on __pthread_exit
> to call exit although for i686 atomic_decrement_and_test will be used and it
> has strong MO.

The same is true for pthread_total counter, it is used to call exit when the last
thread stops. Note while the increment is done optimistically (thread creation
can fail, and then it is decremented again), the decrement resulting in a call to
exit can only happen once even if multiple threads are finishing concurrently
(and that happen once is what is required). 

Cheers,
Wilco



> diff --git a/htl/pt-create.c b/htl/pt-create.c
> index ce52ed9f52210a4e4c7a049ebee817ec9ccfeeb1..14f02cd2b8a19e8581a170dfba2b948ef8304203 100644
> --- a/htl/pt-create.c
> +++ b/htl/pt-create.c
> @@ -228,7 +228,7 @@ __pthread_create_internal (struct __pthread **thread,
>       the number of threads from within the new thread isn't an option
>       since this thread might return and call `pthread_exit' before the
>       new thread runs.  */
> -  atomic_increment (&__pthread_total);
> +  atomic_fetch_add_relaxed (&__pthread_total, 1);
>  
>    /* Store a pointer to this thread in the thread ID lookup table.  We
>       could use __thread_setid, however, we only lock for reading as no

Not sure if this is correct for hurd, __pthread_total is used on __pthread_exit
to call exit although for i686 atomic_decrement_and_test will be used and it
has strong MO.


> diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c
> index aa863c7ea8122ea01d1aa4cffe101bbb7c11270c..3b7e2d434abe8a15145349d1a08a4e706061c74d 100644
> --- a/nptl/nptl_setxid.c
> +++ b/nptl/nptl_setxid.c
> @@ -163,7 +163,7 @@ setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
>    /* If this failed, it must have had not started yet or else exited.  */
>    if (!INTERNAL_SYSCALL_ERROR_P (val))
>      {
> -      atomic_increment (&cmdp->cntr);
> +      atomic_fetch_add_relaxed (&cmdp->cntr, 1);
>        return 1;
>      }
>    else

I am not sure if relaxed MO is correct here, shouldn't it synchronize with the
__nptl_setxid_sighandler ?