Re: [PATCH] misc/tst-clone3: Fix waiting for exited thread.

[Florian Weimer <fweimer at redhat dot com>]

* Stefan Liebler:

> On 02/12/2019 01:53 PM, Stefan Liebler wrote:
>> On 02/11/2019 01:11 PM, Adhemerval Zanella wrote:
>>>
>>>
>>> On 11/02/2019 08:59, Florian Weimer wrote:
>>>> * Stefan Liebler:
>>>>
>>>>> I've first tried to include atomic.h, but it failed building on
>>>>> x86_64. Thus I'm using the c11 atomic load in the updated patch.
>>>>> Okay to commit?
>>>>>
>>>>>
>>>>> As information, I've observed those gcc errors on x86_64:
>>>>> In file included from ../sysdeps/unix/sysv/linux/x86_64/sysdep.h:30,
>>>>>                   from ../sysdeps/x86_64/nptl/tls.h:28,
>>>>>                   from ../sysdeps/x86/atomic-machine.h:23,
>>>>>                   from ../include/atomic.h:50,
>>>>>                   from ../sysdeps/unix/sysv/linux/tst-clone3.c:29:
>>>>> ../sysdeps/unix/sysv/linux/dl-sysdep.h: In function
>>>>> ‘_dl_discover_osversion’:
>>>>> ../sysdeps/unix/sysv/linux/dl-sysdep.h:31:42: error: expected
>>>>> declaration specifiers before ‘attribute_hidden’
>>>>>   extern int _dl_discover_osversion (void) attribute_hidden;
>>>>>                                            ^~~~~~~~~~~~~~~~
>>>>
>>>> That's because the test isn't in tests-internal.
>>>>
>>>>> +    while ((__tid = __atomic_load_n (ctid_ptr,
>>>>> __ATOMIC_ACQUIRE)) != 0)    \
>>>>
>>>> Actually, that's not a C11 atomic construct, but I think it's okay to
>>>> use that here.  (The C11 stuff lives in <stdatomic.h> and should be
>>>> functionally equivalent.)
>>>>
>>>> Sorry, this is a pet peeve of mine.  We have three different atomic
>>>> access facilities that people refer to as C11 atomics: Our own
>>>> <atomic.h>, the GCC __atomic builtins, and <stdatomic.h>.
>>>>
>>>> I still think this contributes to cognitive load, and we should
>>>> eliminate all but one (leaving us with new-style atomics and the old
>>>> macros in <atomic.h>).  The GCC __atomic builtins have the best freely
>>>> available documentation, so they are a natural candidate IMHO.
>>>
>>> It really annoying that C11 standard is not freely available from ISO,
>>> although the working draft is still open [1]. I don't have a strong
>>> opinion, but since there is support in the language itself and they
>>> are fully supported by the compiler I also don't see why not prefer it.
>>>
>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
>>>
>> Then I have a further update of the patch which uses stdatomic.h and
>> atomic_load_explicit.
>
>
> ping
> okay to commit?

I think so.  Patch looks reasonable to me.  Thanks.

Florian