Re: [PATCH] Async signal safe TLS accesses

["Carlos O'Donell" <carlos at redhat dot com>]

On 11/28/2013 07:28 AM, Torvald Riegel wrote:
> I've looked at the synchronization bits in this patch.  Detailed
> comments below, and one general comment:
> 
> Please document the synchronization code you add in more detail.  The
> C11 memory model should be the base model, even though the atomic
> operations we currently have are somewhat different.
> In particular, if using barriers / memory orders (MO) such as "_acq",
> please make sure that you document which ordering they are meant to
> achieve.  (Or, if there are no such barriers or just relaxed MO, please
> document why we don't need a particular ordering.)  For example, if you
> use an acquire barrier / MO, please briefly document with which release
> barriers / MO this is supposed to create happens-before relations.
> Also, the atomic operations that we currently have don't cover all the
> options in the C11 model (for example, we don't have relaxed MO
> read-modify-write ops AFAIR); thus, if you use a glibc atomic operation
> that is stronger than what you'd actually need in the C11 model, please
> document this as well so we can easily change that later on once we
> should have those atomic ops too.

Paul,

I specifically asked Torvald to review this patch, and in general
I agree with Torvald's review.

As a community we're trying to:

* Document parallelism and concurrency in the source code.

  - Lower maintenance burden by documenting enough that we can
    relax the barriers later when we have a more expressive internal
    API for atomic operations.

* Prepare code for the C11 memory model.

  - Try to align new code with the C11 memory model to make it
    easier to maintain this code in the future when we move to
    that model.

All of that means we need a little more documentation in the patch.

Torvald,

Thanks for helping review this patch!

Cheers,
Carlos.