[PATCH] Linux: Match kernel text for SCHED_ macros

[PATCH] Linux: Match kernel text for SCHED_ macros

[Florian Weimer]

This avoids -Werror build issues in strace, which bundles UAPI
headers, but does not include them as system headers.

Fixes commit c444cc1d8335243c5c4e636d6a26c472df85522c
("Linux: Add missing scheduler constants to <sched.h>").

Tested on x86_64-linux-gnu.

---
 sysdeps/unix/sysv/linux/bits/sched.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/bits/sched.h b/sysdeps/unix/sysv/linux/bits/sched.h
index a02cb69de7..f03f5f5ab3 100644
--- a/sysdeps/unix/sysv/linux/bits/sched.h
+++ b/sysdeps/unix/sysv/linux/bits/sched.h
@@ -29,7 +29,7 @@
 #define SCHED_FIFO		1
 #define SCHED_RR		2
 #ifdef __USE_GNU
-# define SCHED_NORMAL		SCHED_OTHER
+# define SCHED_NORMAL		0
 # define SCHED_BATCH		3
 # define SCHED_ISO		4
 # define SCHED_IDLE		5
@@ -48,8 +48,10 @@
 #define SCHED_FLAG_UTIL_CLAMP_MAX	0x40
 
 /* Combinations of sched_flags fields.  */
-#define SCHED_FLAG_KEEP_ALL		0x18
-#define SCHED_FLAG_UTIL_CLAMP		0x60
+#define SCHED_FLAG_KEEP_ALL \
+  (SCHED_FLAG_KEEP_POLICY | SCHED_FLAG_KEEP_PARAMS)
+#define SCHED_FLAG_UTIL_CLAMP \
+  (SCHED_FLAG_UTIL_CLAMP_MIN | SCHED_FLAG_UTIL_CLAMP_MAX)
 
 /* Use "" to work around incorrect macro expansion of the
    __has_include argument (GCC PR 80005).  */

base-commit: c5dd659f22058bf9b371ab1cba07631f1206c674

Re: [PATCH] Linux: Match kernel text for SCHED_ macros

[Carlos O'Donell]

On 10/25/24 9:00 AM, Florian Weimer wrote:
> This avoids -Werror build issues in strace, which bundles UAPI
> headers, but does not include them as system headers.
> 
> Fixes commit c444cc1d8335243c5c4e636d6a26c472df85522c
> ("Linux: Add missing scheduler constants to <sched.h>").
> 
> Tested on x86_64-linux-gnu.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  sysdeps/unix/sysv/linux/bits/sched.h | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/bits/sched.h b/sysdeps/unix/sysv/linux/bits/sched.h
> index a02cb69de7..f03f5f5ab3 100644
> --- a/sysdeps/unix/sysv/linux/bits/sched.h
> +++ b/sysdeps/unix/sysv/linux/bits/sched.h
> @@ -29,7 +29,7 @@
>  #define SCHED_FIFO		1
>  #define SCHED_RR		2
>  #ifdef __USE_GNU
> -# define SCHED_NORMAL		SCHED_OTHER
> +# define SCHED_NORMAL		0

OK. Correct.

>  # define SCHED_BATCH		3
>  # define SCHED_ISO		4
>  # define SCHED_IDLE		5
> @@ -48,8 +48,10 @@
>  #define SCHED_FLAG_UTIL_CLAMP_MAX	0x40
>  
>  /* Combinations of sched_flags fields.  */
> -#define SCHED_FLAG_KEEP_ALL		0x18
> -#define SCHED_FLAG_UTIL_CLAMP		0x60
> +#define SCHED_FLAG_KEEP_ALL \
> +  (SCHED_FLAG_KEEP_POLICY | SCHED_FLAG_KEEP_PARAMS)
> +#define SCHED_FLAG_UTIL_CLAMP \
> +  (SCHED_FLAG_UTIL_CLAMP_MIN | SCHED_FLAG_UTIL_CLAMP_MAX)

OK.

>  
>  /* Use "" to work around incorrect macro expansion of the
>     __has_include argument (GCC PR 80005).  */
> 
> base-commit: c5dd659f22058bf9b371ab1cba07631f1206c674
> 

-- 
Cheers,
Carlos.

Re: [PATCH] Linux: Match kernel text for SCHED_ macros

[Dmitry V. Levin]

On Fri, Oct 25, 2024 at 03:00:50PM +0200, Florian Weimer wrote:
> This avoids -Werror build issues in strace, which bundles UAPI
> headers, but does not include them as system headers.
> 
> Fixes commit c444cc1d8335243c5c4e636d6a26c472df85522c
> ("Linux: Add missing scheduler constants to <sched.h>").
> 
> Tested on x86_64-linux-gnu.

Looks like commit c444cc1d8335243c5c4e636d6a26c472df85522c indeed broke
compilation of several strace tests that use the following pattern:
 
 #include <sched.h>
 #include <linux/sched.h>
  
I suppose this patch fixes that issue, however, I don't see what
"does not include them as system headers" could possibly mean.


-- 
ldv

Re: [PATCH] Linux: Match kernel text for SCHED_ macros

[Florian Weimer]

* Dmitry V. Levin:

> On Fri, Oct 25, 2024 at 03:00:50PM +0200, Florian Weimer wrote:
>> This avoids -Werror build issues in strace, which bundles UAPI
>> headers, but does not include them as system headers.
>> 
>> Fixes commit c444cc1d8335243c5c4e636d6a26c472df85522c
>> ("Linux: Add missing scheduler constants to <sched.h>").
>> 
>> Tested on x86_64-linux-gnu.
>
> Looks like commit c444cc1d8335243c5c4e636d6a26c472df85522c indeed broke
> compilation of several strace tests that use the following pattern:
>  
>  #include <sched.h>
>  #include <linux/sched.h>
>   
> I suppose this patch fixes that issue, however, I don't see what
> "does not include them as system headers" could possibly mean.

It's the difference between -I and -isystem.  Only the latter suppresses
warnings by default.  The default search path roughly behaves as
specified with -isystem.

Thanks,
Florian

Re: [PATCH] Linux: Match kernel text for SCHED_ macros

[Dmitry V. Levin]

On Sat, Oct 26, 2024 at 01:13:53AM +0200, Florian Weimer wrote:
> * Dmitry V. Levin:
> > On Fri, Oct 25, 2024 at 03:00:50PM +0200, Florian Weimer wrote:
> >> This avoids -Werror build issues in strace, which bundles UAPI
> >> headers, but does not include them as system headers.
> >> 
> >> Fixes commit c444cc1d8335243c5c4e636d6a26c472df85522c
> >> ("Linux: Add missing scheduler constants to <sched.h>").
> >> 
> >> Tested on x86_64-linux-gnu.
> >
> > Looks like commit c444cc1d8335243c5c4e636d6a26c472df85522c indeed broke
> > compilation of several strace tests that use the following pattern:
> >  
> >  #include <sched.h>
> >  #include <linux/sched.h>
> >   
> > I suppose this patch fixes that issue, however, I don't see what
> > "does not include them as system headers" could possibly mean.
> 
> It's the difference between -I and -isystem.  Only the latter suppresses
> warnings by default.  The default search path roughly behaves as
> specified with -isystem.

I see.  Would you suggest that in case when strace uses bundled Linux UAPI
headers, they are included via -isystem rather than -I?


-- 
ldv

Re: [PATCH] Linux: Match kernel text for SCHED_ macros

[Florian Weimer]

* Dmitry V. Levin:

> On Sat, Oct 26, 2024 at 01:13:53AM +0200, Florian Weimer wrote:
>> * Dmitry V. Levin:
>> > On Fri, Oct 25, 2024 at 03:00:50PM +0200, Florian Weimer wrote:
>> >> This avoids -Werror build issues in strace, which bundles UAPI
>> >> headers, but does not include them as system headers.
>> >> 
>> >> Fixes commit c444cc1d8335243c5c4e636d6a26c472df85522c
>> >> ("Linux: Add missing scheduler constants to <sched.h>").
>> >> 
>> >> Tested on x86_64-linux-gnu.
>> >
>> > Looks like commit c444cc1d8335243c5c4e636d6a26c472df85522c indeed broke
>> > compilation of several strace tests that use the following pattern:
>> >  
>> >  #include <sched.h>
>> >  #include <linux/sched.h>
>> >   
>> > I suppose this patch fixes that issue, however, I don't see what
>> > "does not include them as system headers" could possibly mean.
>> 
>> It's the difference between -I and -isystem.  Only the latter suppresses
>> warnings by default.  The default search path roughly behaves as
>> specified with -isystem.
>
> I see.  Would you suggest that in case when strace uses bundled Linux UAPI
> headers, they are included via -isystem rather than -I?

I don't have a preference.  Both approaches are valid.  The additional
cross-checking by -I (requiring the same tokens) between headers might
useful to glibc and others.  Suppressing the warnings with -isystem
could help avoid -Werror errors that people just building strace for
their system cannot easily solve.

Thanks,
Florian