[1/1] uapi: Don't include <linux/sysinfo.h> in <linux/kernel.h>

Message ID 20201001195231.17226-1-petr.vorel@gmail.com
State Not applicable
Headers
Series [1/1] uapi: Don't include <linux/sysinfo.h> in <linux/kernel.h> |

Commit Message

Petr Vorel Oct. 1, 2020, 7:52 p.m. UTC
  + update code where needed (include <linux/sysinfo.h> in code which
included <linux/kernel.h> only to get struct sysinfo or SI_LOAD_SHIFT).

The reason is to avoid indirect <linux/sysinfo.h> include when using
some network headers: <linux/netlink.h> or others [1] ->
<linux/kernel.h> -> <linux/sysinfo.h>.

This indirect include causes redefinition of struct sysinfo when
included both <sys/sysinfo.h> and some of network headers:

In file included from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/kernel.h:5,
                 from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/netlink.h:5,
                 from ../include/tst_netlink.h:14,
                 from tst_crypto.c:13:
x86_64-buildroot-linux-musl/sysroot/usr/include/linux/sysinfo.h:8:8: error: redefinition of ‘struct sysinfo’
 struct sysinfo {
        ^~~~~~~
In file included from ../include/tst_safe_macros.h:15,
                 from ../include/tst_test.h:93,
                 from tst_crypto.c:11:
x86_64-buildroot-linux-musl/sysroot/usr/include/sys/sysinfo.h:10:8: note: originally defined here

[1] or <linux/sysctl.h>, <linux/ethtool.h>, <linux/mroute6.h>, <linux/ethtool.h>

Suggested-by: Rich Felker <dalias@aerifal.cx>
Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
Hi,

this looks to be long standing problem: python-psutil [2], iproute2 [3],
even for glibc in the past [4] and it tried to be solved before [5].

This will require glibc fix after:

+++ sysdeps/unix/sysv/linux/sys/sysinfo.h
@@ -21,7 +21,7 @@
 #include <features.h>

 /* Get sysinfo structure from kernel header.  */
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>

 __BEGIN_DECLS


Kind regards,
Petr

[2] https://git.buildroot.net/buildroot/commit/?id=ab930190d7754b7f5fdda115b2069d00f1320861
[3] http://patchwork.ozlabs.org/project/netdev/patch/602128d22db86bd67e11dec8fe40a73832c222c9.1559230347.git.baruch@tkos.co.il/
[4] https://www.mail-archive.com/busybox@busybox.net/msg15363.html
[5] https://lore.kernel.org/patchwork/patch/449607/

 drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +-
 fs/eventpoll.c                      | 2 +-
 fs/proc/meminfo.c                   | 2 +-
 include/linux/mm.h                  | 1 +
 include/uapi/linux/kernel.h         | 2 --
 kernel/debug/kdb/kdb_main.c         | 2 +-
 kernel/sys.c                        | 2 +-
 mm/page_alloc.c                     | 2 +-
 net/ipv4/inetpeer.c                 | 2 +-
 net/rds/ib_recv.c                   | 2 +-
 10 files changed, 9 insertions(+), 10 deletions(-)
  

Comments

Rich Felker Oct. 1, 2020, 8:11 p.m. UTC | #1
On Thu, Oct 01, 2020 at 09:52:31PM +0200, Petr Vorel wrote:
> + update code where needed (include <linux/sysinfo.h> in code which
> included <linux/kernel.h> only to get struct sysinfo or SI_LOAD_SHIFT).
> 
> The reason is to avoid indirect <linux/sysinfo.h> include when using
> some network headers: <linux/netlink.h> or others [1] ->
> <linux/kernel.h> -> <linux/sysinfo.h>.
> 
> This indirect include causes redefinition of struct sysinfo when
> included both <sys/sysinfo.h> and some of network headers:
> 
> In file included from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/kernel.h:5,
>                  from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/netlink.h:5,
>                  from ../include/tst_netlink.h:14,
>                  from tst_crypto.c:13:
> x86_64-buildroot-linux-musl/sysroot/usr/include/linux/sysinfo.h:8:8: error: redefinition of ‘struct sysinfo’
>  struct sysinfo {
>         ^~~~~~~
> In file included from ../include/tst_safe_macros.h:15,
>                  from ../include/tst_test.h:93,
>                  from tst_crypto.c:11:
> x86_64-buildroot-linux-musl/sysroot/usr/include/sys/sysinfo.h:10:8: note: originally defined here
> 
> [1] or <linux/sysctl.h>, <linux/ethtool.h>, <linux/mroute6.h>, <linux/ethtool.h>
> 
> Suggested-by: Rich Felker <dalias@aerifal.cx>
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> ---
> Hi,
> 
> this looks to be long standing problem: python-psutil [2], iproute2 [3],
> even for glibc in the past [4] and it tried to be solved before [5].
> 
> This will require glibc fix after:

You can't do this; it breaks the existing contract with glibc. New
kernel headers can't force a glibc upgrade. You just have to get rid
of use of <linux/kernel.h> elsewhere in the uapi headers. It was a
mistake that <linux/sysinfo.h> was ever separated out of
<linux/kernel.h> since it didn't (and couldn't) fix the contract that
<linux/kernel.h> exposes struct sysinfo (and that it's misnamed). But
it's no big deal. This can all be fixed without any breakage anywhere
just by not using it.

Rich
  
Petr Vorel Oct. 1, 2020, 8:27 p.m. UTC | #2
Hi Rich,

> On Thu, Oct 01, 2020 at 09:52:31PM +0200, Petr Vorel wrote:
> > + update code where needed (include <linux/sysinfo.h> in code which
> > included <linux/kernel.h> only to get struct sysinfo or SI_LOAD_SHIFT).

> > The reason is to avoid indirect <linux/sysinfo.h> include when using
> > some network headers: <linux/netlink.h> or others [1] ->
> > <linux/kernel.h> -> <linux/sysinfo.h>.

> > This indirect include causes redefinition of struct sysinfo when
> > included both <sys/sysinfo.h> and some of network headers:

> > In file included from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/kernel.h:5,
> >                  from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/netlink.h:5,
> >                  from ../include/tst_netlink.h:14,
> >                  from tst_crypto.c:13:
> > x86_64-buildroot-linux-musl/sysroot/usr/include/linux/sysinfo.h:8:8: error: redefinition of ‘struct sysinfo’
> >  struct sysinfo {
> >         ^~~~~~~
> > In file included from ../include/tst_safe_macros.h:15,
> >                  from ../include/tst_test.h:93,
> >                  from tst_crypto.c:11:
> > x86_64-buildroot-linux-musl/sysroot/usr/include/sys/sysinfo.h:10:8: note: originally defined here

> > [1] or <linux/sysctl.h>, <linux/ethtool.h>, <linux/mroute6.h>, <linux/ethtool.h>

> > Suggested-by: Rich Felker <dalias@aerifal.cx>
> > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > ---
> > Hi,

> > this looks to be long standing problem: python-psutil [2], iproute2 [3],
> > even for glibc in the past [4] and it tried to be solved before [5].

> > This will require glibc fix after:

> You can't do this; it breaks the existing contract with glibc. New
> kernel headers can't force a glibc upgrade.
Right, got that.

> You just have to get rid
> of use of <linux/kernel.h> elsewhere in the uapi headers. It was a
> mistake that <linux/sysinfo.h> was ever separated out of
> <linux/kernel.h> since it didn't (and couldn't) fix the contract that
> <linux/kernel.h> exposes struct sysinfo (and that it's misnamed). But
> it's no big deal. This can all be fixed without any breakage anywhere
> just by not using it.
Back to your original suggestion to move the alignment macros to a separate
header. I was trying to avoid it not sure if introducing new header is
acceptable, but we'll see.

> Rich

Kind regards,
Petr
  
Rich Felker Oct. 1, 2020, 9:52 p.m. UTC | #3
On Thu, Oct 01, 2020 at 10:27:03PM +0200, Petr Vorel wrote:
> Hi Rich,
> 
> > On Thu, Oct 01, 2020 at 09:52:31PM +0200, Petr Vorel wrote:
> > > + update code where needed (include <linux/sysinfo.h> in code which
> > > included <linux/kernel.h> only to get struct sysinfo or SI_LOAD_SHIFT).
> 
> > > The reason is to avoid indirect <linux/sysinfo.h> include when using
> > > some network headers: <linux/netlink.h> or others [1] ->
> > > <linux/kernel.h> -> <linux/sysinfo.h>.
> 
> > > This indirect include causes redefinition of struct sysinfo when
> > > included both <sys/sysinfo.h> and some of network headers:
> 
> > > In file included from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/kernel.h:5,
> > >                  from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/netlink.h:5,
> > >                  from ../include/tst_netlink.h:14,
> > >                  from tst_crypto.c:13:
> > > x86_64-buildroot-linux-musl/sysroot/usr/include/linux/sysinfo.h:8:8: error: redefinition of ‘struct sysinfo’
> > >  struct sysinfo {
> > >         ^~~~~~~
> > > In file included from ../include/tst_safe_macros.h:15,
> > >                  from ../include/tst_test.h:93,
> > >                  from tst_crypto.c:11:
> > > x86_64-buildroot-linux-musl/sysroot/usr/include/sys/sysinfo.h:10:8: note: originally defined here
> 
> > > [1] or <linux/sysctl.h>, <linux/ethtool.h>, <linux/mroute6.h>, <linux/ethtool.h>
> 
> > > Suggested-by: Rich Felker <dalias@aerifal.cx>
> > > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > > ---
> > > Hi,
> 
> > > this looks to be long standing problem: python-psutil [2], iproute2 [3],
> > > even for glibc in the past [4] and it tried to be solved before [5].
> 
> > > This will require glibc fix after:
> 
> > You can't do this; it breaks the existing contract with glibc. New
> > kernel headers can't force a glibc upgrade.
> Right, got that.
> 
> > You just have to get rid
> > of use of <linux/kernel.h> elsewhere in the uapi headers. It was a
> > mistake that <linux/sysinfo.h> was ever separated out of
> > <linux/kernel.h> since it didn't (and couldn't) fix the contract that
> > <linux/kernel.h> exposes struct sysinfo (and that it's misnamed). But
> > it's no big deal. This can all be fixed without any breakage anywhere
> > just by not using it.
> Back to your original suggestion to move the alignment macros to a separate
> header. I was trying to avoid it not sure if introducing new header is
> acceptable, but we'll see.

Isn't there already another similar header with that type of macro
that they belong in?

Rich
  
Petr Vorel Oct. 1, 2020, 10:29 p.m. UTC | #4
Hi,

> On Thu, Oct 01, 2020 at 10:27:03PM +0200, Petr Vorel wrote:
> > Hi Rich,

> > > On Thu, Oct 01, 2020 at 09:52:31PM +0200, Petr Vorel wrote:
> > > > + update code where needed (include <linux/sysinfo.h> in code which
> > > > included <linux/kernel.h> only to get struct sysinfo or SI_LOAD_SHIFT).

> > > > The reason is to avoid indirect <linux/sysinfo.h> include when using
> > > > some network headers: <linux/netlink.h> or others [1] ->
> > > > <linux/kernel.h> -> <linux/sysinfo.h>.

> > > > This indirect include causes redefinition of struct sysinfo when
> > > > included both <sys/sysinfo.h> and some of network headers:

> > > > In file included from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/kernel.h:5,
> > > >                  from x86_64-buildroot-linux-musl/sysroot/usr/include/linux/netlink.h:5,
> > > >                  from ../include/tst_netlink.h:14,
> > > >                  from tst_crypto.c:13:
> > > > x86_64-buildroot-linux-musl/sysroot/usr/include/linux/sysinfo.h:8:8: error: redefinition of ‘struct sysinfo’
> > > >  struct sysinfo {
> > > >         ^~~~~~~
> > > > In file included from ../include/tst_safe_macros.h:15,
> > > >                  from ../include/tst_test.h:93,
> > > >                  from tst_crypto.c:11:
> > > > x86_64-buildroot-linux-musl/sysroot/usr/include/sys/sysinfo.h:10:8: note: originally defined here

> > > > [1] or <linux/sysctl.h>, <linux/ethtool.h>, <linux/mroute6.h>, <linux/ethtool.h>

> > > > Suggested-by: Rich Felker <dalias@aerifal.cx>
> > > > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > > > ---
> > > > Hi,

> > > > this looks to be long standing problem: python-psutil [2], iproute2 [3],
> > > > even for glibc in the past [4] and it tried to be solved before [5].

> > > > This will require glibc fix after:

> > > You can't do this; it breaks the existing contract with glibc. New
> > > kernel headers can't force a glibc upgrade.
> > Right, got that.

> > > You just have to get rid
> > > of use of <linux/kernel.h> elsewhere in the uapi headers. It was a
> > > mistake that <linux/sysinfo.h> was ever separated out of
> > > <linux/kernel.h> since it didn't (and couldn't) fix the contract that
> > > <linux/kernel.h> exposes struct sysinfo (and that it's misnamed). But
> > > it's no big deal. This can all be fixed without any breakage anywhere
> > > just by not using it.
> > Back to your original suggestion to move the alignment macros to a separate
> > header. I was trying to avoid it not sure if introducing new header is
> > acceptable, but we'll see.

> Isn't there already another similar header with that type of macro
> that they belong in?
The only one I've found is const.h. Not sure it'd be better to move things
there.

Kind regards,
Petr

> Rich
  

Patch

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 8062bd99add8..6c84c92e5112 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -43,7 +43,7 @@ 
  * USA.
  */
 
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/module.h>
 #include <linux/errno.h>
 #include <linux/init.h>
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 8107e06d7f6f..a4c2e199f9ea 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -7,7 +7,7 @@ 
  */
 
 #include <linux/init.h>
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/sched/signal.h>
 #include <linux/fs.h>
 #include <linux/file.h>
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index 887a5532e449..c80e42e1eaf9 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/fs.h>
 #include <linux/init.h>
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/mm.h>
 #include <linux/hugetlb.h>
 #include <linux/mman.h>
diff --git a/include/linux/mm.h b/include/linux/mm.h
index b2f370f0b420..4ac1af86db59 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -7,6 +7,7 @@ 
 #ifdef __KERNEL__
 
 #include <linux/mmdebug.h>
+#include <linux/sysinfo.h>
 #include <linux/gfp.h>
 #include <linux/bug.h>
 #include <linux/list.h>
diff --git a/include/uapi/linux/kernel.h b/include/uapi/linux/kernel.h
index 0ff8f7477847..e95f8cd801cd 100644
--- a/include/uapi/linux/kernel.h
+++ b/include/uapi/linux/kernel.h
@@ -2,8 +2,6 @@ 
 #ifndef _UAPI_LINUX_KERNEL_H
 #define _UAPI_LINUX_KERNEL_H
 
-#include <linux/sysinfo.h>
-
 /*
  * 'kernel.h' contains some often-used function prototypes etc
  */
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 5c7949061671..3193b9dd90e8 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -14,7 +14,7 @@ 
 #include <linux/ctype.h>
 #include <linux/types.h>
 #include <linux/string.h>
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/kmsg_dump.h>
 #include <linux/reboot.h>
 #include <linux/sched.h>
diff --git a/kernel/sys.c b/kernel/sys.c
index ab6c409b1159..9a3ee7ba84a7 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -16,7 +16,7 @@ 
 #include <linux/kmod.h>
 #include <linux/perf_event.h>
 #include <linux/resource.h>
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/workqueue.h>
 #include <linux/capability.h>
 #include <linux/device.h>
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fab5e97dc9ca..42e14c5392a6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -24,7 +24,7 @@ 
 #include <linux/jiffies.h>
 #include <linux/memblock.h>
 #include <linux/compiler.h>
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/kasan.h>
 #include <linux/module.h>
 #include <linux/suspend.h>
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index ff327a62c9ce..1c7385dab1c4 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -15,7 +15,7 @@ 
 #include <linux/random.h>
 #include <linux/timer.h>
 #include <linux/time.h>
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/mm.h>
 #include <linux/net.h>
 #include <linux/workqueue.h>
diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index 694d411dc72f..81425a2863e0 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -30,7 +30,7 @@ 
  * SOFTWARE.
  *
  */
-#include <linux/kernel.h>
+#include <linux/sysinfo.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
 #include <linux/dma-mapping.h>