[Mesa-dev] [PATCH v2] c11/threads: Use PTHREAD_MUTEX_RECURSIVE by default

[Emil Velikov]

On 03/02/15 17:51, Felix Janda wrote:
> Emil Velikov wrote:
>> On 3 February 2015 at 02:53, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> Hi Felix,
>>>
> 
> Thanks for the review.
> 
>>> On 2 February 2015 at 19:04, Felix Janda <felix.janda at posteo.de> wrote:
>>>> Previously PTHREAD_MUTEX_RECURSIVE_NP had been used on linux for
>>>> compatibility with old glibc. Since mesa defines __GNU_SOURCE__
>>>> on linux PTHREAD_MUTEX_RECURSIVE is also available since at least
>>>> 1998. So we can unconditionally use the portable version
>>>> PTHREAD_MUTEX_RECURSIVE.
>>>> ---
>>>> Previous patch didn't work as intended since on glibc these are
>>>> part of an enum and not defines.
>>>>
>>> From a quick look at {e,}glibc sources [1]
>>>
>>>    PTHREAD_MUTEX_RECURSIVE_NP,
>>>    ...
>>>    #if defined __USE_UNIX98 || defined __USE_XOPEN2K8
>>>    ...
>>>    PTHREAD_MUTEX_RECURSIVE = PTHREAD_MUTEX_RECURSIVE_NP,
>>>    ...
>>>
>>> Which does not seem to have relation with _GNU_SOURCE_ as the commit
>>> mentions. Note I don't even pretend to know the convoluted web of
>>> macros, so I could be a bit off :-)
>>>
> 
> _GNU_SOURCE basically stands for "enable everything". I've checked that
> for glibc 2.1 it defined _XOPEN_SOURCE=500 which in turn defines
> __USE_UNIX98.
> 
>> Upon closer look things could be fine, as
>>
>>   linux/gnu: _GNU_SOURCE sets _POSIX_C_SOURCE 200809L.
>> With the latter of which guarding the __USE_XOPEN2K8  definition.
>>
>>   cygwin: (hmm does it even set __linux*) we set _XOPEN_SOURCE=700.
>> Which is the guard for __USE_XOPEN2K8.
> 
> http://nadeausoftware.com/articles/2012/01/c_c_tip_how_use_compiler_predefined_macros_detect_operating_system
> 
> seems to indicate that it doesn't. I don't think that anything but
> glibc has PTHREAD_MUTEX_RECURSIVE_NP.
> 
>> If one wants to be extra careful we can change
>>   #if defined(__linux__) || defined(__linux)
>> to check for the said old versions of glibc, but I believe we should be safe.
> 
> #if defined(__GLIBC__) is fine, too. Unless you care about hurd:
> https://bugzilla.gnome.org/show_bug.cgi?id=377066
> 
I believe we do, although I'm not 100% sure.

> Something along this line is necessary (and sufficient) to compile
> mesa unmodified with musl libc.
> 
With all that said, does anyone else have input on this ?

I'm planning to add the following tags and push this some time next week.

Cc: "10.5" <mesa-stable at lists.freedesktop.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88534
Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>


Thanks
Emil