New way for syncing with kernel headers

New way for syncing with kernel headers

[Florian Weimer]

Would this be an acceptable way to obtain definitions from the Linux 
headers in places where namespace cleanliness is not a must?

/* Obtain the definitions of the MEMBARRIER_CMD_* constants.  */

#include <linux/version.h>
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 14, 0)
# include <linux/membarrier.h>
#else

/* Definitions from Linux 4.14 follow.  */

enum membarrier_cmd
{
   MEMBARRIER_CMD_QUERY = 0,
   MEMBARRIER_CMD_SHARED = 1,
   MEMBARRIER_CMD_PRIVATE_EXPEDITED = 8,
   MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = 16,
};

#endif


The idea is to have a built-in fallback definition in case the installed 
kernel headers are too old.  But for newer kernels, the UAPI header is 
used, to avoid duplicate definitions and a need for maintaining our own 
copy continuously.

The inline copy of the definitions reflects what is needed for building 
glibc itself (including tests) and the manual.  But if the end user 
installs newer kernel headers, their definitions are immediately 
available for use with glibc-based applications.  Eventually, we can 
remove the inline copy once the minimum kernel version for building 
moves past the baseline requirement.

Header include ordering does not matter because <linux/membarrier.h> has 
its own include guard.

Has this already been tried?  Do you see any downsides?

Thanks,
Florian

Re: New way for syncing with kernel headers

[Jonathan Nieder]

Hi,

Florian Weimer wrote:

> Would this be an acceptable way to obtain definitions from the Linux headers
> in places where namespace cleanliness is not a must?
>
> /* Obtain the definitions of the MEMBARRIER_CMD_* constants.  */
>
> #include <linux/version.h>
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 14, 0)
> # include <linux/membarrier.h>
> #else
>
> /* Definitions from Linux 4.14 follow.  */
>
> enum membarrier_cmd
> {
>   MEMBARRIER_CMD_QUERY = 0,
>   MEMBARRIER_CMD_SHARED = 1,
>   MEMBARRIER_CMD_PRIVATE_EXPEDITED = 8,
>   MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = 16,
> };
>
> #endif
>
>
> The idea is to have a built-in fallback definition in case the installed
> kernel headers are too old.  But for newer kernels, the UAPI header is used,
> to avoid duplicate definitions and a need for maintaining our own copy
> continuously.
>
> The inline copy of the definitions reflects what is needed for building
> glibc itself (including tests) and the manual.  But if the end user installs
> newer kernel headers, their definitions are immediately available for use
> with glibc-based applications.  Eventually, we can remove the inline copy
> once the minimum kernel version for building moves past the baseline
> requirement.

Sounds good to me!  Some quick notes:

- I don't see any uses of LINUX_VERSION_CODE in glibc today except in
  the configure script.  Is there a reason to prefer
  __LINUX_KERNEL_VERSION instead (maybe for a more stable namespace,
  even in contexts where namespace polution is not an issue)?

- I'd worry about the LINUX_VERSION_CODE < 4.14 case bitrotting.  Is
  there an easy way to ensure it gets test-compiled e.g. as part of
  build-many-glibcs?

Thanks,
Jonathan

Re: New way for syncing with kernel headers

[Florian Weimer]

On 12/08/2017 08:49 PM, Jonathan Nieder wrote:

> - I don't see any uses of LINUX_VERSION_CODE in glibc today except in
>    the configure script.  Is there a reason to prefer
>    __LINUX_KERNEL_VERSION instead (maybe for a more stable namespace,
>    even in contexts where namespace polution is not an issue)?

We need something which is defined in an UAPI header today.

__LINUX_KERNEL_VERSION is an internal glibc macro for a different 
purpose: it indicates the minimum (assumed) kernel version.

> - I'd worry about the LINUX_VERSION_CODE < 4.14 case bitrotting.  Is
>    there an easy way to ensure it gets test-compiled e.g. as part of
>    build-many-glibcs?

Easy is relative: Build with older kernel headers.

Thanks,
Florian

Re: New way for syncing with kernel headers

[Jonathan Nieder]

Florian Weimer wrote:
> On 12/08/2017 08:49 PM, Jonathan Nieder wrote:

>> - I don't see any uses of LINUX_VERSION_CODE in glibc today except in
>>    the configure script.  Is there a reason to prefer
>>    __LINUX_KERNEL_VERSION instead (maybe for a more stable namespace,
>>    even in contexts where namespace polution is not an issue)?
>
> We need something which is defined in an UAPI header today.
>
> __LINUX_KERNEL_VERSION is an internal glibc macro for a different purpose:
> it indicates the minimum (assumed) kernel version.

Yes, that makes sense.  I think the answer to my implied question of why
we weren't using LINUX_VERSION_CODE is that we just hadn't needed it yet.

For what it's worth, your proposal still sounds good to me.

>> - I'd worry about the LINUX_VERSION_CODE < 4.14 case bitrotting.  Is
>>    there an easy way to ensure it gets test-compiled e.g. as part of
>>    build-many-glibcs?
>
> Easy is relative: Build with older kernel headers.

I was hoping for something more lightweight than an additional full
glibc build, but that certainly works in a pinch.

Thanks,
Jonathan

Re: New way for syncing with kernel headers

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Fri, Dec 08, 2017 at 02:21:27PM +0100, Florian Weimer wrote:
> Would this be an acceptable way to obtain definitions from the Linux 
> headers in places where namespace cleanliness is not a must?
> 
> /* Obtain the definitions of the MEMBARRIER_CMD_* constants.  */
> 
> #include <linux/version.h>
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 14, 0)
> # include <linux/membarrier.h>
> #else
> 
> /* Definitions from Linux 4.14 follow.  */
> 
> enum membarrier_cmd
> {
>    MEMBARRIER_CMD_QUERY = 0,
>    MEMBARRIER_CMD_SHARED = 1,
>    MEMBARRIER_CMD_PRIVATE_EXPEDITED = 8,
>    MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = 16,
> };
> 
> #endif

Isn't it going to break when userspace uses e.g.
LINUX_VERSION_CODE == KERNEL_VERSION(4, 13, 0)
and includes <linux/membarrier.h> after this header?


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: New way for syncing with kernel headers

[Florian Weimer]

On 12/08/2017 09:32 PM, Dmitry V. Levin wrote:
> On Fri, Dec 08, 2017 at 02:21:27PM +0100, Florian Weimer wrote:
>> Would this be an acceptable way to obtain definitions from the Linux
>> headers in places where namespace cleanliness is not a must?
>>
>> /* Obtain the definitions of the MEMBARRIER_CMD_* constants.  */
>>
>> #include <linux/version.h>
>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 14, 0)
>> # include <linux/membarrier.h>
>> #else
>>
>> /* Definitions from Linux 4.14 follow.  */
>>
>> enum membarrier_cmd
>> {
>>     MEMBARRIER_CMD_QUERY = 0,
>>     MEMBARRIER_CMD_SHARED = 1,
>>     MEMBARRIER_CMD_PRIVATE_EXPEDITED = 8,
>>     MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = 16,
>> };
>>
>> #endif
> 
> Isn't it going to break when userspace uses e.g.
> LINUX_VERSION_CODE == KERNEL_VERSION(4, 13, 0)
> and includes <linux/membarrier.h> after this header?

Ugh, good point.

If I use 4.3 as the version guard (where <linux/membarrier.h> was 
introduced first), someone with 4.13 kernel headers will not able to 
compile a test using MEMBARRIER_CMD_PRIVATE_EXPEDITED.  So that doesn't 
work, either.

But if we copy the header contents, like we currently do in most cases, 
you can include the UAPI header afterwards, either.  So maybe the 
proposed approach is still an improvement because you don't have to 
include the UAPI header at all if your glibc version is sufficiently 
recent because the glibc header will give you the UAPI header plus 
whatever system call wrappers glibc provides.

Thanks,
Florian

Re: New way for syncing with kernel headers

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 2151 bytes --]

On Fri, Dec 08, 2017 at 10:03:25PM +0100, Florian Weimer wrote:
> On 12/08/2017 09:32 PM, Dmitry V. Levin wrote:
> > On Fri, Dec 08, 2017 at 02:21:27PM +0100, Florian Weimer wrote:
> >> Would this be an acceptable way to obtain definitions from the Linux
> >> headers in places where namespace cleanliness is not a must?
> >>
> >> /* Obtain the definitions of the MEMBARRIER_CMD_* constants.  */
> >>
> >> #include <linux/version.h>
> >> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 14, 0)
> >> # include <linux/membarrier.h>
> >> #else
> >>
> >> /* Definitions from Linux 4.14 follow.  */
> >>
> >> enum membarrier_cmd
> >> {
> >>     MEMBARRIER_CMD_QUERY = 0,
> >>     MEMBARRIER_CMD_SHARED = 1,
> >>     MEMBARRIER_CMD_PRIVATE_EXPEDITED = 8,
> >>     MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = 16,
> >> };
> >>
> >> #endif
> > 
> > Isn't it going to break when userspace uses e.g.
> > LINUX_VERSION_CODE == KERNEL_VERSION(4, 13, 0)
> > and includes <linux/membarrier.h> after this header?
> 
> Ugh, good point.
> 
> If I use 4.3 as the version guard (where <linux/membarrier.h> was 
> introduced first), someone with 4.13 kernel headers will not able to 
> compile a test using MEMBARRIER_CMD_PRIVATE_EXPEDITED.  So that doesn't 
> work, either.
> 
> But if we copy the header contents, like we currently do in most cases, 
> you can include the UAPI header afterwards, either.  So maybe the 

s/you can/you cannot/

> proposed approach is still an improvement because you don't have to 
> include the UAPI header at all if your glibc version is sufficiently 
> recent because the glibc header will give you the UAPI header plus 
> whatever system call wrappers glibc provides.

Yes, the proposed approach is an improvement.

Relatively portable software would have to be able to include
the UAPI header, too.  Maybe we should document the least troublesome
way of implementing this, e.g.

#if defined HAVE_SYS_MEMBARRIER_H
# include <sys/membarrier.h>
#elif defined HAVE_LINUX_MEMBARRIER_H
# include <linux/membarrier.h>
#else
# error membarrier API is not available
#endif


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]