Stable Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues
@ 2020-05-21 15:53 Joel Fernandes (Google)
  2020-05-21 15:55 ` Joel Fernandes
  0 siblings, 1 reply; 7+ messages in thread
From: Joel Fernandes (Google) @ 2020-05-21 15:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google),
	matthewb, jsbarnes, vapier, christian, vpillai, vineethrp,
	peterz, stable, gregkh

On a modern Linux distro, compiling the following program fails:
 #include<stdlib.h>
 #include<stdint.h>
 #include<pthread.h>
 #include<linux/sched/types.h>

 void main() {
         struct sched_attr sa;

         return;
 }

with:
/usr/include/linux/sched/types.h:8:8: \
			error: redefinition of ‘struct sched_param’
    8 | struct sched_param {
      |        ^~~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/bits/sched.h:74,
                 from /usr/include/sched.h:43,
                 from /usr/include/pthread.h:23,
                 from /tmp/s.c:4:
/usr/include/x86_64-linux-gnu/bits/types/struct_sched_param.h:23:8:
note: originally defined here
   23 | struct sched_param
      |        ^~~~~~~~~~~

This is also causing a problem on using sched_attr Chrome. The issue is
sched_param is already provided by glibc.

Guard the kernel's UAPI definition of sched_param with __KERNEL__ so
that userspace can compile.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 include/uapi/linux/sched/types.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/sched/types.h b/include/uapi/linux/sched/types.h
index c852153ddb0d3..1f10d935a63fe 100644
--- a/include/uapi/linux/sched/types.h
+++ b/include/uapi/linux/sched/types.h
@@ -4,9 +4,11 @@
 
 #include <linux/types.h>
 
+#if defined(__KERNEL__)
 struct sched_param {
 	int sched_priority;
 };
+#endif
 
 #define SCHED_ATTR_SIZE_VER0	48	/* sizeof first published struct */
 #define SCHED_ATTR_SIZE_VER1	56	/* add: util_{min,max} */
-- 
2.26.2.761.g0e0b3e54be-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues
  2020-05-21 15:53 [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues Joel Fernandes (Google)
@ 2020-05-21 15:55 ` Joel Fernandes
  2020-05-22 13:13   ` Christian Brauner
  2020-05-22 14:02   ` Peter Zijlstra
  0 siblings, 2 replies; 7+ messages in thread
From: Joel Fernandes @ 2020-05-21 15:55 UTC (permalink / raw)
  To: LKML
  Cc: Matthew Blecker, Jesse Barnes, Mike Frysinger, Christian Brauner,
	Vineeth Remanan Pillai, vineethrp, Peter Zijlstra, stable,
	Greg Kroah-Hartman

On Thu, May 21, 2020 at 11:53 AM Joel Fernandes (Google)
<joel@joelfernandes.org> wrote:
>
> On a modern Linux distro, compiling the following program fails:
>  #include<stdlib.h>
>  #include<stdint.h>
>  #include<pthread.h>
>  #include<linux/sched/types.h>
>
>  void main() {
>          struct sched_attr sa;
>
>          return;
>  }
>
> with:
> /usr/include/linux/sched/types.h:8:8: \
>                         error: redefinition of ‘struct sched_param’
>     8 | struct sched_param {
>       |        ^~~~~~~~~~~
> In file included from /usr/include/x86_64-linux-gnu/bits/sched.h:74,
>                  from /usr/include/sched.h:43,
>                  from /usr/include/pthread.h:23,
>                  from /tmp/s.c:4:
> /usr/include/x86_64-linux-gnu/bits/types/struct_sched_param.h:23:8:
> note: originally defined here
>    23 | struct sched_param
>       |        ^~~~~~~~~~~
>
> This is also causing a problem on using sched_attr Chrome. The issue is
> sched_param is already provided by glibc.
>
> Guard the kernel's UAPI definition of sched_param with __KERNEL__ so
> that userspace can compile.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>

If it is more preferable, another option is to move sched_param to
include/linux/sched/types.h

 - Joel


> ---
>  include/uapi/linux/sched/types.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/uapi/linux/sched/types.h b/include/uapi/linux/sched/types.h
> index c852153ddb0d3..1f10d935a63fe 100644
> --- a/include/uapi/linux/sched/types.h
> +++ b/include/uapi/linux/sched/types.h
> @@ -4,9 +4,11 @@
>
>  #include <linux/types.h>
>
> +#if defined(__KERNEL__)
>  struct sched_param {
>         int sched_priority;
>  };
> +#endif
>
>  #define SCHED_ATTR_SIZE_VER0   48      /* sizeof first published struct */
>  #define SCHED_ATTR_SIZE_VER1   56      /* add: util_{min,max} */
> --
> 2.26.2.761.g0e0b3e54be-goog
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues
  2020-05-21 15:55 ` Joel Fernandes
@ 2020-05-22 13:13   ` Christian Brauner
  2020-05-22 13:38     ` Joel Fernandes
  2020-05-22 14:02   ` Peter Zijlstra
  1 sibling, 1 reply; 7+ messages in thread
From: Christian Brauner @ 2020-05-22 13:13 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: LKML, Matthew Blecker, Jesse Barnes, Mike Frysinger,
	Christian Brauner, Vineeth Remanan Pillai, vineethrp,
	Peter Zijlstra, stable, Greg Kroah-Hartman

On Thu, May 21, 2020 at 11:55:21AM -0400, Joel Fernandes wrote:
> On Thu, May 21, 2020 at 11:53 AM Joel Fernandes (Google)
> <joel@joelfernandes.org> wrote:
> >
> > On a modern Linux distro, compiling the following program fails:
> >  #include<stdlib.h>
> >  #include<stdint.h>
> >  #include<pthread.h>
> >  #include<linux/sched/types.h>
> >
> >  void main() {
> >          struct sched_attr sa;
> >
> >          return;
> >  }
> >
> > with:
> > /usr/include/linux/sched/types.h:8:8: \
> >                         error: redefinition of ‘struct sched_param’
> >     8 | struct sched_param {
> >       |        ^~~~~~~~~~~
> > In file included from /usr/include/x86_64-linux-gnu/bits/sched.h:74,
> >                  from /usr/include/sched.h:43,
> >                  from /usr/include/pthread.h:23,
> >                  from /tmp/s.c:4:
> > /usr/include/x86_64-linux-gnu/bits/types/struct_sched_param.h:23:8:
> > note: originally defined here
> >    23 | struct sched_param
> >       |        ^~~~~~~~~~~
> >
> > This is also causing a problem on using sched_attr Chrome. The issue is
> > sched_param is already provided by glibc.
> >
> > Guard the kernel's UAPI definition of sched_param with __KERNEL__ so
> > that userspace can compile.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> 
> If it is more preferable, another option is to move sched_param to
> include/linux/sched/types.h

Might it be worth Ccing libc-alpha here? Seems like one of those classic
header conflicts.

Christian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues
  2020-05-22 13:13   ` Christian Brauner
@ 2020-05-22 13:38     ` Joel Fernandes
  2020-05-22 13:46       ` Christian Brauner
  0 siblings, 1 reply; 7+ messages in thread
From: Joel Fernandes @ 2020-05-22 13:38 UTC (permalink / raw)
  To: Christian Brauner
  Cc: LKML, Matthew Blecker, Jesse Barnes, Mike Frysinger,
	Christian Brauner, Vineeth Remanan Pillai, vineethrp,
	Peter Zijlstra, stable, Greg Kroah-Hartman

On Fri, May 22, 2020 at 03:13:55PM +0200, Christian Brauner wrote:
> On Thu, May 21, 2020 at 11:55:21AM -0400, Joel Fernandes wrote:
> > On Thu, May 21, 2020 at 11:53 AM Joel Fernandes (Google)
> > <joel@joelfernandes.org> wrote:
> > >
> > > On a modern Linux distro, compiling the following program fails:
> > >  #include<stdlib.h>
> > >  #include<stdint.h>
> > >  #include<pthread.h>
> > >  #include<linux/sched/types.h>
> > >
> > >  void main() {
> > >          struct sched_attr sa;
> > >
> > >          return;
> > >  }
> > >
> > > with:
> > > /usr/include/linux/sched/types.h:8:8: \
> > >                         error: redefinition of ‘struct sched_param’
> > >     8 | struct sched_param {
> > >       |        ^~~~~~~~~~~
> > > In file included from /usr/include/x86_64-linux-gnu/bits/sched.h:74,
> > >                  from /usr/include/sched.h:43,
> > >                  from /usr/include/pthread.h:23,
> > >                  from /tmp/s.c:4:
> > > /usr/include/x86_64-linux-gnu/bits/types/struct_sched_param.h:23:8:
> > > note: originally defined here
> > >    23 | struct sched_param
> > >       |        ^~~~~~~~~~~
> > >
> > > This is also causing a problem on using sched_attr Chrome. The issue is
> > > sched_param is already provided by glibc.
> > >
> > > Guard the kernel's UAPI definition of sched_param with __KERNEL__ so
> > > that userspace can compile.
> > >
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > 
> > If it is more preferable, another option is to move sched_param to
> > include/linux/sched/types.h
> 
> Might it be worth Ccing libc-alpha here? Seems like one of those classic
> header conflicts.

sched_param is defined by POSIX from my reading of the manpage. Is the kernel
supposed to define it in the UAPI at all? I guarded it with __KERNEL__ as you
can see.

Resent with libc-alpha CC'd per your suggestion.

thanks,

 - Joel



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues
  2020-05-22 13:38     ` Joel Fernandes
@ 2020-05-22 13:46       ` Christian Brauner
  0 siblings, 0 replies; 7+ messages in thread
From: Christian Brauner @ 2020-05-22 13:46 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: LKML, Matthew Blecker, Jesse Barnes, Mike Frysinger,
	Christian Brauner, Vineeth Remanan Pillai, vineethrp,
	Peter Zijlstra, stable, Greg Kroah-Hartman

On Fri, May 22, 2020 at 09:38:16AM -0400, Joel Fernandes wrote:
> On Fri, May 22, 2020 at 03:13:55PM +0200, Christian Brauner wrote:
> > On Thu, May 21, 2020 at 11:55:21AM -0400, Joel Fernandes wrote:
> > > On Thu, May 21, 2020 at 11:53 AM Joel Fernandes (Google)
> > > <joel@joelfernandes.org> wrote:
> > > >
> > > > On a modern Linux distro, compiling the following program fails:
> > > >  #include<stdlib.h>
> > > >  #include<stdint.h>
> > > >  #include<pthread.h>
> > > >  #include<linux/sched/types.h>
> > > >
> > > >  void main() {
> > > >          struct sched_attr sa;
> > > >
> > > >          return;
> > > >  }
> > > >
> > > > with:
> > > > /usr/include/linux/sched/types.h:8:8: \
> > > >                         error: redefinition of ‘struct sched_param’
> > > >     8 | struct sched_param {
> > > >       |        ^~~~~~~~~~~
> > > > In file included from /usr/include/x86_64-linux-gnu/bits/sched.h:74,
> > > >                  from /usr/include/sched.h:43,
> > > >                  from /usr/include/pthread.h:23,
> > > >                  from /tmp/s.c:4:
> > > > /usr/include/x86_64-linux-gnu/bits/types/struct_sched_param.h:23:8:
> > > > note: originally defined here
> > > >    23 | struct sched_param
> > > >       |        ^~~~~~~~~~~
> > > >
> > > > This is also causing a problem on using sched_attr Chrome. The issue is
> > > > sched_param is already provided by glibc.
> > > >
> > > > Guard the kernel's UAPI definition of sched_param with __KERNEL__ so
> > > > that userspace can compile.
> > > >
> > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > 
> > > If it is more preferable, another option is to move sched_param to
> > > include/linux/sched/types.h
> > 
> > Might it be worth Ccing libc-alpha here? Seems like one of those classic
> > header conflicts.
> 
> sched_param is defined by POSIX from my reading of the manpage. Is the kernel
> supposed to define it in the UAPI at all? I guarded it with __KERNEL__ as you
> can see.

Your patch is fine of course. :) It's just that conflicts like this
have happened before. Another conflict is e.g. in wait.h where the
kernel has #define P_* and libc has an enum for P_* and it's not at all
guaranteed that they are identical. Plus, sometimes the order of header
inclusion matters because of things like this (or something like this).
That's why having it seen on libc-alpha might help prevent accidentaly
causing bugs where you now include a header that gives you a different
definition than you expected.

> 
> Resent with libc-alpha CC'd per your suggestion.

Thanks!

Christian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues
  2020-05-21 15:55 ` Joel Fernandes
  2020-05-22 13:13   ` Christian Brauner
@ 2020-05-22 14:02   ` Peter Zijlstra
  2020-05-22 19:08     ` Joel Fernandes
  1 sibling, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2020-05-22 14:02 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: LKML, Matthew Blecker, Jesse Barnes, Mike Frysinger,
	Christian Brauner, Vineeth Remanan Pillai, vineethrp, stable,
	Greg Kroah-Hartman

On Thu, May 21, 2020 at 11:55:21AM -0400, Joel Fernandes wrote:
> On Thu, May 21, 2020 at 11:53 AM Joel Fernandes (Google)
> <joel@joelfernandes.org> wrote:
> >
> > On a modern Linux distro, compiling the following program fails:
> >  #include<stdlib.h>
> >  #include<stdint.h>
> >  #include<pthread.h>
> >  #include<linux/sched/types.h>
> >
> >  void main() {
> >          struct sched_attr sa;
> >
> >          return;
> >  }
> >
> > with:
> > /usr/include/linux/sched/types.h:8:8: \
> >                         error: redefinition of ‘struct sched_param’
> >     8 | struct sched_param {
> >       |        ^~~~~~~~~~~
> > In file included from /usr/include/x86_64-linux-gnu/bits/sched.h:74,
> >                  from /usr/include/sched.h:43,
> >                  from /usr/include/pthread.h:23,
> >                  from /tmp/s.c:4:
> > /usr/include/x86_64-linux-gnu/bits/types/struct_sched_param.h:23:8:
> > note: originally defined here
> >    23 | struct sched_param
> >       |        ^~~~~~~~~~~
> >
> > This is also causing a problem on using sched_attr Chrome. The issue is
> > sched_param is already provided by glibc.
> >
> > Guard the kernel's UAPI definition of sched_param with __KERNEL__ so
> > that userspace can compile.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> 
> If it is more preferable, another option is to move sched_param to
> include/linux/sched/types.h

Yeah, not sure. Ingo, you got a preference?

Also, this very much misses a Fixes tag.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues
  2020-05-22 14:02   ` Peter Zijlstra
@ 2020-05-22 19:08     ` Joel Fernandes
  0 siblings, 0 replies; 7+ messages in thread
From: Joel Fernandes @ 2020-05-22 19:08 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: LKML, Matthew Blecker, Jesse Barnes, Mike Frysinger,
	Christian Brauner, Vineeth Remanan Pillai, vineethrp, stable,
	Greg Kroah-Hartman

On Fri, May 22, 2020 at 04:02:26PM +0200, Peter Zijlstra wrote:
> On Thu, May 21, 2020 at 11:55:21AM -0400, Joel Fernandes wrote:
> > On Thu, May 21, 2020 at 11:53 AM Joel Fernandes (Google)
> > <joel@joelfernandes.org> wrote:
> > >
> > > On a modern Linux distro, compiling the following program fails:
> > >  #include<stdlib.h>
> > >  #include<stdint.h>
> > >  #include<pthread.h>
> > >  #include<linux/sched/types.h>
> > >
> > >  void main() {
> > >          struct sched_attr sa;
> > >
> > >          return;
> > >  }
> > >
> > > with:
> > > /usr/include/linux/sched/types.h:8:8: \
> > >                         error: redefinition of ‘struct sched_param’
> > >     8 | struct sched_param {
> > >       |        ^~~~~~~~~~~
> > > In file included from /usr/include/x86_64-linux-gnu/bits/sched.h:74,
> > >                  from /usr/include/sched.h:43,
> > >                  from /usr/include/pthread.h:23,
> > >                  from /tmp/s.c:4:
> > > /usr/include/x86_64-linux-gnu/bits/types/struct_sched_param.h:23:8:
> > > note: originally defined here
> > >    23 | struct sched_param
> > >       |        ^~~~~~~~~~~
> > >
> > > This is also causing a problem on using sched_attr Chrome. The issue is
> > > sched_param is already provided by glibc.
> > >
> > > Guard the kernel's UAPI definition of sched_param with __KERNEL__ so
> > > that userspace can compile.
> > >
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > 
> > If it is more preferable, another option is to move sched_param to
> > include/linux/sched/types.h
> 
> Yeah, not sure. Ingo, you got a preference?
> 
> Also, this very much misses a Fixes tag.

Ok, will add Fixe: tag as follows:

Fixes: e2d1e2aec572a ("sched/headers: Move various ABI definitions to <uapi/linux/sched/types.h>")

thanks,

 - Joel


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 15:53 [PATCH RFC] sched/headers: Fix sched_setattr userspace compilation issues Joel Fernandes (Google)
2020-05-21 15:55 ` Joel Fernandes
2020-05-22 13:13   ` Christian Brauner
2020-05-22 13:38     ` Joel Fernandes
2020-05-22 13:46       ` Christian Brauner
2020-05-22 14:02   ` Peter Zijlstra
2020-05-22 19:08     ` Joel Fernandes

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git