Re: [PATCH v1 1/1] kernel.h: Drop inclusion in bitmap.h
On Sun, Mar 28, 2021 at 12:23 AM Al Viro wrote: > > On Fri, Mar 26, 2021 at 07:03:47PM +0200, Andy Shevchenko wrote: > > The bitmap.h header is used in a lot of code around the kernel. > > Besides that it includes kernel.h which sometimes makes a loop. > > How much of the kernel does *not* end up pulling kernel.h anyway, > making all subsequent includes fast no-ops? > > Realistically, any C compiler is going to recognize the case when > included file has contents of form > > #ifndef > #define > > #endif > > where contain no exposed #else, #elif or #endif and > remember that subsequent includes of that file should be ignored > unless becomes undefined. The problem here is not in C compiler, but in many unneeded loops that make header hell dependencies. For example, how you may move bitmap_zalloc() from C-file to the header? Currently it's impossible. And bitmap.h here is only a tip of an iceberg. kerne.h is a dump of everything that even has nothing in common at all. We may still have it, but in my new code I prefer to include only the headers that I want to use, without the bulk of unneeded kernel code. > AFAICS, on e.g. allmodconfig amd64 build it's (at most - there > might've been false positives) 131 files out of ~3; on > defconfig it's 55 out of ~2300. How much does your patch > change that? I'm not sure I understand what you mean here. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/1] kernel.h: Drop inclusion in bitmap.h
On Fri, Mar 26, 2021 at 07:03:47PM +0200, Andy Shevchenko wrote: > The bitmap.h header is used in a lot of code around the kernel. > Besides that it includes kernel.h which sometimes makes a loop. How much of the kernel does *not* end up pulling kernel.h anyway, making all subsequent includes fast no-ops? Realistically, any C compiler is going to recognize the case when included file has contents of form #ifndef #define #endif where contain no exposed #else, #elif or #endif and remember that subsequent includes of that file should be ignored unless becomes undefined. AFAICS, on e.g. allmodconfig amd64 build it's (at most - there might've been false positives) 131 files out of ~3; on defconfig it's 55 out of ~2300. How much does your patch change that?
Re: [PATCH v1 1/1] kernel.h: Drop inclusion in bitmap.h
On Fri, Mar 26, 2021 at 07:03:47PM +0200, Andy Shevchenko wrote: > The bitmap.h header is used in a lot of code around the kernel. > Besides that it includes kernel.h which sometimes makes a loop. > > Break the loop by introducing align.h, including it in kernel.h > and bitmap.h followed by replacing kernel.h with limits.h. > > Signed-off-by: Andy Shevchenko Can you give an example of such dependency? Nevertheless, Acked-by: Yury Norov > --- > include/linux/align.h | 15 +++ > include/linux/bitmap.h | 3 ++- > include/linux/kernel.h | 9 + > 3 files changed, 18 insertions(+), 9 deletions(-) > create mode 100644 include/linux/align.h > > diff --git a/include/linux/align.h b/include/linux/align.h > new file mode 100644 > index ..2b4acec7b95a > --- /dev/null > +++ b/include/linux/align.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_ALIGN_H > +#define _LINUX_ALIGN_H > + > +#include > + > +/* @a is a power of 2 value */ > +#define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > +#define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a)) > +#define __ALIGN_MASK(x, mask)__ALIGN_KERNEL_MASK((x), (mask)) > +#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), > (a))) > +#define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a))) > +#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) > + > +#endif /* _LINUX_ALIGN_H */ > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h > index 70a932470b2d..6cbcd9d9edd2 100644 > --- a/include/linux/bitmap.h > +++ b/include/linux/bitmap.h > @@ -4,10 +4,11 @@ > > #ifndef __ASSEMBLY__ > > +#include > #include > #include > +#include > #include > -#include > > /* > * bitmaps provide bit arrays that consume one or more unsigned > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 5b7ed6dc99ac..09035ac67d4b 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -3,6 +3,7 @@ > #define _LINUX_KERNEL_H > > #include > +#include > #include > #include > #include > @@ -30,14 +31,6 @@ > */ > #define REPEAT_BYTE(x) ((~0ul / 0xff) * (x)) > > -/* @a is a power of 2 value */ > -#define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > -#define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a)) > -#define __ALIGN_MASK(x, mask)__ALIGN_KERNEL_MASK((x), (mask)) > -#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), > (a))) > -#define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a))) > -#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) > - > /* generic data direction definitions */ > #define READ 0 > #define WRITE1 > -- > 2.30.2
[PATCH v1 1/1] kernel.h: Drop inclusion in bitmap.h
The bitmap.h header is used in a lot of code around the kernel. Besides that it includes kernel.h which sometimes makes a loop. Break the loop by introducing align.h, including it in kernel.h and bitmap.h followed by replacing kernel.h with limits.h. Signed-off-by: Andy Shevchenko --- include/linux/align.h | 15 +++ include/linux/bitmap.h | 3 ++- include/linux/kernel.h | 9 + 3 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 include/linux/align.h diff --git a/include/linux/align.h b/include/linux/align.h new file mode 100644 index ..2b4acec7b95a --- /dev/null +++ b/include/linux/align.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_ALIGN_H +#define _LINUX_ALIGN_H + +#include + +/* @a is a power of 2 value */ +#define ALIGN(x, a)__ALIGN_KERNEL((x), (a)) +#define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a)) +#define __ALIGN_MASK(x, mask) __ALIGN_KERNEL_MASK((x), (mask)) +#define PTR_ALIGN(p, a)((typeof(p))ALIGN((unsigned long)(p), (a))) +#define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a))) +#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) + +#endif /* _LINUX_ALIGN_H */ diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 70a932470b2d..6cbcd9d9edd2 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -4,10 +4,11 @@ #ifndef __ASSEMBLY__ +#include #include #include +#include #include -#include /* * bitmaps provide bit arrays that consume one or more unsigned diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 5b7ed6dc99ac..09035ac67d4b 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -3,6 +3,7 @@ #define _LINUX_KERNEL_H #include +#include #include #include #include @@ -30,14 +31,6 @@ */ #define REPEAT_BYTE(x) ((~0ul / 0xff) * (x)) -/* @a is a power of 2 value */ -#define ALIGN(x, a)__ALIGN_KERNEL((x), (a)) -#define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a)) -#define __ALIGN_MASK(x, mask) __ALIGN_KERNEL_MASK((x), (mask)) -#define PTR_ALIGN(p, a)((typeof(p))ALIGN((unsigned long)(p), (a))) -#define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a))) -#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) - /* generic data direction definitions */ #define READ 0 #define WRITE 1 -- 2.30.2
