Re: [PATCH 1/2] nptl: Remove __ASSUME_SET_ROBUST_LIST

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 20/04/2017 09:52, Florian Weimer wrote:
> On 04/20/2017 02:50 PM, Adhemerval Zanella wrote:
>>
>>
>> On 19/04/2017 15:09, Florian Weimer wrote:
>>> On 04/19/2017 07:38 PM, Adhemerval Zanella wrote:
>>>> On Wed, Apr 19, 2017 at 12:28 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>> On 04/19/2017 04:17 PM, Adhemerval Zanella wrote:
>>>>>>
>>>>>> The problem is kernel supports for some architecture depends of kernel
>>>>>> config
>>>>>> and the underlying hardware revision/model.
>>>>>
>>>>>
>>>>> I understand that.  But I have seen a lot of use of robust mutexes lately,
>>>>> and I'm sure these developers aren't aware that the mutexes aren't portable
>>>>> across GNU/Linux (the Linux architecture subset supported by glibc).
>>>>>
>>>>> I expect it's like the missing accept4 system call—you need to provide the
>>>>> set_robust_list system call in the kernel if you want to a working, modern
>>>>> system.
>>>>
>>>> But my point is with current minimum supported kernel version for some
>>>> architectures
>>>> we can't simple assume set_robust_list support and even bumping minimum kernel
>>>> version for some architectures also do not solve the issue (on mips
>>>> for instance).
>>>
>>> Hmm, maybe you are right, and we have to keep things this way for now.
>>>
>>> But I don't like that your patch reintroduces the conditional code.  You could use
>>>
>>> #define __set_robust_list_avail 1
>>>
>>> for the __ASSUME_SET_ROBUST_LIST case to reduce clutter, and let GCC do the rest.
>>
>> Rechecking what you meant, I am not following you here: the idea is to have
>> __set_robust_list_avail being a runtime variable and get rid of the _ASSUME
>> define. How using a define would help in this way?
> 
> I suggest to keep the __ASSUME_SET_ROBUST_LIST define but concentrate its use to a single place by making __set_robust_list_avail available unconditionally, but as a constant for  __ASSUME_SET_ROBUST_LIST.  This is the pattern we used elsewhere (e.g. for have_cloexec).

But what I am suggesting is exactly to get rid of __ASSUME_SET_ROBUST_LIST to
simplify the conditionals and have it defined at runtime instead of having
each architecture define whether it supports or not set_robust_list. What you
are suggesting is basically current behaviour.