Re: [PATCH] Linux: Implement membarrier function

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

On 05/12/2018 17:53, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 05/12/2018 16:11, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> On 05/12/2018 12:51, Florian Weimer wrote:
>>>>> * Adhemerval Zanella:
>>>>>
>>>>>> If we are replicating the values, meaning the idea is to keep it sync at least
>>>>>> when we have the minimum supported kernel of 4.16, why just not add a comment
>>>>>> to add linux/membarrier.h once the minimum supported kernel provides this
>>>>>> header and not rely on linux/membarrier.h?
>>>>>
>>>>> This way, we can compile the test with any supported kernel headers for
>>>>> glibc.
>>>>>
>>>>> If we defer to <linux/membarrier.h> unconditionally, we cannot build the
>>>>> test with all kernel headers.  The current approach definitely makes the
>>>>> test case much cleaner.
>>>>>
>>>>
>>>> My point is exactly to *not* rely on kernel headers.
>>>
>>> I thought there was a general desire to move in the opposite direction,
>>> avoiding copying declarations and definitions in clean/new headers that
>>> are dedicated to a specific purpose?
>>
>> I don't recall this discussion, do you have the link?
> 
> I will try to find it.
> 
>> In any case I still think mixing header inclusion with the definition
>> duplication is just a double effort, it will still require syncing the
>> definitions on each kernel release.
> 
> I think that's not actually true.  We only need to do this if we need
> newer definitions for building glibc itself (including the test suite).
> 
>>> I don't think we can add much value by copying the contents of those
>>> headers.  We can even relax the version check if we use __has_include
>>> (which has to be remain optional in installed headers, of course).
> 
>> This example give us why trying to relying in kernel header to provide
>> glibc exported interfaces are at least tricky, imho.  The issues I can
>> think of are:
>>
>>   - The header would require to be include safe in all possible releases
>>     and for all possible permutations, which means if something is broken 
>>     we need to rely on kernel to actually fix it.
> 
> I'm not too worried about this for new headers with a dedicated purpose.
> Using the UAPI will actually make things work in more cases; see below.
> 
>>   - glibc might provide different semantic to users depending of the
>>     underlying installed kernel header version.  It means for two
>>     identical glibc versions, some programs might fail to build 
>>     depending of the installed kernel version.
> 
> Well, that already happens with the duplicated header approach if users
> want to use UAPI headers, too.  It requires complicated synchronization
> to make it work.

But at least glibc is not imposing it, if user want to use kernel uapi
header he will explicit include it and it is up to kernel provide a sane
implementation.

> 
> We can tweak the conditional for the header inclusion further, like
> this:
> 
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 16, 0) \
> +  || __glibc_has_include (<linux/membarrier.h>) \
> +# include <linux/membarrier.h>
> +#else
> 
> With GCC 5 and later, this would always prefer the kernel header if
> available.  There would never be a conflict between the definitions,
> irrespective of header file inclusion order.
> 
> What do you think?

In any case I don't think my suggestion should be a blocker, we already rely 
on linux header for some cases and it seems I am alone in trying to decouple
glibc from kernel headers.