1. Patchew
  2. linux
  3. [PATCH v2 0/2] shrink lib/string.i via IWYU
  4. View patch

[PATCH v2 2/2] lib/string: shrink lib/string.i via IWYU

tanzirh@google.com posted 2 patches 2 years ago
There is a newer version of this series
[PATCH v2 2/2] lib/string: shrink lib/string.i via IWYU
Posted by tanzirh@google.com 2 years ago 
This diff uses an open source tool include-what-you-use (IWYU) to modify
the include list changing indirect includes to direct includes.
IWYU is implemented using the IWYUScripts github repository which is a tool that is
currently undergoing development. These changes seek to improve build times.

This change to lib/string.c resulted in a preprocessed size of
lib/string.i from 26371 lines to 5259 lines (-80%) for the x86
defconfig.

Link: https://github.com/ClangBuiltLinux/IWYUScripts

Signed-off-by: Tanzir Hasan <tanzirh@google.com>
---
 lib/string.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index be26623953d2..7fe1acefb1a1 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -16,16 +16,16 @@
 
 #define __NO_FORTIFY
 #include <linux/types.h>
-#include <linux/string.h>
-#include <linux/ctype.h>
-#include <linux/kernel.h>
-#include <linux/export.h>
+#include <linux/bits.h>
 #include <linux/bug.h>
 #include <linux/errno.h>
-#include <linux/slab.h>
-
+#include <asm/rwonce.h>
+#include <linux/linkage.h>
+#include <linux/stddef.h>
+#include <vdso/limits.h>
+#include <linux/string.h>
+#include <linux/ctype.h>
 #include <asm/unaligned.h>
-#include <asm/byteorder.h>
 #include <asm/word-at-a-time.h>
 #include <asm/page.h>
 

-- 
2.43.0.472.g3155946c3a-goog
Re: [PATCH v2 2/2] lib/string: shrink lib/string.i via IWYU
Posted by Justin Stitt 1 year, 11 months ago 
Hi,

On Thu, Dec 14, 2023 at 09:06:13PM +0000, tanzirh@google.com wrote:
> This diff uses an open source tool include-what-you-use (IWYU) to modify
> the include list changing indirect includes to direct includes.
> IWYU is implemented using the IWYUScripts github repository which is a tool that is
> currently undergoing development. These changes seek to improve build times.
>
> This change to lib/string.c resulted in a preprocessed size of
> lib/string.i from 26371 lines to 5259 lines (-80%) for the x86
> defconfig.
>

Tanzir, I wonder if you could include some of the symbol names used for
some of these more esoteric headers. Let me describe what I mean:

Andy talks about "why <vdso/...>" and perhaps some comments (in your
patch message, not in the source itself) about which symbols are being
used from these headers would serve useful. I believe IWYU can generate
this information and should clear up some confusion or lead to better
suggestions from reviewers if we understand why a header is being
included.

At any rate, this builds for me doing randconfigs on x86_64 with these
KCONFIG_SEEDs
1: 0x3DD9D136
2: 0xB4440EE4
3: 0x98778270
4: 0x8C237F26
5: 0x244F8A64
6: 0x5A5C5E5C
7: 0xA77896BC
8: 0x9B5FF0D5
9: 0x24F23F6A
10: 0x35C0A107

I applied your patch on top of 5db8752c3b81bd33.

Tested-by: Justin Stitt <justinstitt@google.com>
> Link: https://github.com/ClangBuiltLinux/IWYUScripts
>
> Signed-off-by: Tanzir Hasan <tanzirh@google.com>
> ---
>  lib/string.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/lib/string.c b/lib/string.c
> index be26623953d2..7fe1acefb1a1 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -16,16 +16,16 @@
>
>  #define __NO_FORTIFY
>  #include <linux/types.h>
> -#include <linux/string.h>
> -#include <linux/ctype.h>
> -#include <linux/kernel.h>
> -#include <linux/export.h>
> +#include <linux/bits.h>
>  #include <linux/bug.h>
>  #include <linux/errno.h>
> -#include <linux/slab.h>
> -
> +#include <asm/rwonce.h>
> +#include <linux/linkage.h>
> +#include <linux/stddef.h>
> +#include <vdso/limits.h>
> +#include <linux/string.h>
> +#include <linux/ctype.h>
>  #include <asm/unaligned.h>
> -#include <asm/byteorder.h>
>  #include <asm/word-at-a-time.h>
>  #include <asm/page.h>
>
>
> --
> 2.43.0.472.g3155946c3a-goog
>
Thanks
Justin
Re: [PATCH v2 2/2] lib/string: shrink lib/string.i via IWYU
Posted by Kees Cook 1 year, 11 months ago 
On Tue, Jan 09, 2024 at 09:49:10PM +0000, Justin Stitt wrote:
> On Thu, Dec 14, 2023 at 09:06:13PM +0000, tanzirh@google.com wrote:
> > This diff uses an open source tool include-what-you-use (IWYU) to modify
> > the include list changing indirect includes to direct includes.
> > IWYU is implemented using the IWYUScripts github repository which is a tool that is
> > currently undergoing development. These changes seek to improve build times.
> >
> > This change to lib/string.c resulted in a preprocessed size of
> > lib/string.i from 26371 lines to 5259 lines (-80%) for the x86
> > defconfig.
> >
> 
> Tanzir, I wonder if you could include some of the symbol names used for
> some of these more esoteric headers. Let me describe what I mean:
> 
> Andy talks about "why <vdso/...>" and perhaps some comments (in your
> patch message, not in the source itself) about which symbols are being
> used from these headers would serve useful. I believe IWYU can generate
> this information and should clear up some confusion or lead to better
> suggestions from reviewers if we understand why a header is being
> included.

If there aren't any other objections, I'd like to pick this up in my
tree after the merge window closes.

> 
> At any rate, this builds for me doing randconfigs on x86_64 with these
> KCONFIG_SEEDs
> 1: 0x3DD9D136
> 2: 0xB4440EE4
> 3: 0x98778270
> 4: 0x8C237F26
> 5: 0x244F8A64
> 6: 0x5A5C5E5C
> 7: 0xA77896BC
> 8: 0x9B5FF0D5
> 9: 0x24F23F6A
> 10: 0x35C0A107
> 
> I applied your patch on top of 5db8752c3b81bd33.
> 
> Tested-by: Justin Stitt <justinstitt@google.com>

Thanks for the testing!

-Kees

> > Link: https://github.com/ClangBuiltLinux/IWYUScripts
> >
> > Signed-off-by: Tanzir Hasan <tanzirh@google.com>
> > ---
> >  lib/string.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/string.c b/lib/string.c
> > index be26623953d2..7fe1acefb1a1 100644
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -16,16 +16,16 @@
> >
> >  #define __NO_FORTIFY
> >  #include <linux/types.h>
> > -#include <linux/string.h>
> > -#include <linux/ctype.h>
> > -#include <linux/kernel.h>
> > -#include <linux/export.h>
> > +#include <linux/bits.h>
> >  #include <linux/bug.h>
> >  #include <linux/errno.h>
> > -#include <linux/slab.h>
> > -
> > +#include <asm/rwonce.h>
> > +#include <linux/linkage.h>
> > +#include <linux/stddef.h>
> > +#include <vdso/limits.h>
> > +#include <linux/string.h>
> > +#include <linux/ctype.h>
> >  #include <asm/unaligned.h>
> > -#include <asm/byteorder.h>
> >  #include <asm/word-at-a-time.h>
> >  #include <asm/page.h>
> >
> >
> > --
> > 2.43.0.472.g3155946c3a-goog
> >
> Thanks
> Justin

-- 
Kees Cook
Re: [PATCH v2 2/2] lib/string: shrink lib/string.i via IWYU
Posted by Andy Shevchenko 2 years ago 
On Thu, Dec 14, 2023 at 09:06:13PM +0000, tanzirh@google.com wrote:
> This diff uses an open source tool include-what-you-use (IWYU) to modify
> the include list changing indirect includes to direct includes.
> IWYU is implemented using the IWYUScripts github repository which is a tool that is
> currently undergoing development. These changes seek to improve build times.
> 
> This change to lib/string.c resulted in a preprocessed size of
> lib/string.i from 26371 lines to 5259 lines (-80%) for the x86
> defconfig.

> Link: https://github.com/ClangBuiltLinux/IWYUScripts
> 
> Signed-off-by: Tanzir Hasan <tanzirh@google.com>

Tag block should not have blank line(s).

>  #include <linux/types.h>
> -#include <linux/string.h>
> -#include <linux/ctype.h>
> -#include <linux/kernel.h>
> -#include <linux/export.h>
> +#include <linux/bits.h>
>  #include <linux/bug.h>
>  #include <linux/errno.h>
> -#include <linux/slab.h>
> -
> +#include <asm/rwonce.h>
> +#include <linux/linkage.h>
> +#include <linux/stddef.h>
> +#include <vdso/limits.h>
> +#include <linux/string.h>
> +#include <linux/ctype.h>
>  #include <asm/unaligned.h>
> -#include <asm/byteorder.h>
>  #include <asm/word-at-a-time.h>
>  #include <asm/page.h>

My gosh, this becomes unreadable!

Please make sure you go with groups of headers from more generic to more
particular, i.e.

linux/*

asm/*

vdso/*

BUT, why the heck vdso? It should be rather uapi/ or regular linux/.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/2] lib/string: shrink lib/string.i via IWYU
Posted by Kees Cook 2 years ago 
On Thu, Dec 14, 2023 at 09:06:13PM +0000, tanzirh@google.com wrote:
> This diff uses an open source tool include-what-you-use (IWYU) to modify
> the include list changing indirect includes to direct includes.
> IWYU is implemented using the IWYUScripts github repository which is a tool that is
> currently undergoing development. These changes seek to improve build times.
> 
> This change to lib/string.c resulted in a preprocessed size of
> lib/string.i from 26371 lines to 5259 lines (-80%) for the x86
> defconfig.
> 
> Link: https://github.com/ClangBuiltLinux/IWYUScripts
> 

nit: Please drop this blank line to keep your tags together.

> Signed-off-by: Tanzir Hasan <tanzirh@google.com>

I'm glad to see such a big difference with just a little header
tweaking. I look forward to more like this!

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.