This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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: Fix sem_post race (bug 14532)


Some style and typo issues.  I don't have any feedback about the substance
of the comments.  It also seems worthwhile to have some wiki page somewhere
about concurrency issues, references for terminology, and conventions about
how we comment the code for such issues.

> --- a/nptl/sysdeps/unix/sysv/linux/sem_post.c
> +++ b/nptl/sysdeps/unix/sysv/linux/sem_post.c
> @@ -30,6 +30,13 @@ __new_sem_post (sem_t *sem)
>  {
>    struct new_sem *isem = (struct new_sem *) sem;
>  
> +  /* Use a CAS loop to increment the semaphore's value.  We need release
> +     memory order (MO) on the CAS so that it synchronizes with the other CAS
> +     used by threads waiting on the semaphore (which has acquire MO); this
> +     ensures that everything we did before posting the semaphore happens
> +     before the waiters can continue.  We do not need additional acquire MO

"need an additional"

> +  /* This seq_cst fence is necessary for the Dekker-style synchronization used
> +     here.  This fence and the respective fence in__new_sem_wait() are totally

Missing space before "__new_sem_wait".  Please do not use "foo()" in
comments, documentation, or log entries in libc.  The parens are not part
of the function name.  Just use the name alone.

> +     ordered, which in turn prevents cyclic reads-from/sequenced-before
> +     dependencies between the store/load pairs;  here, this pair is the prior

Extra space after ; here.


Thanks,
Roland


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