| Message ID | 20231214-libstringheader-v2-2-0f195dcff204@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | shrink lib/string.i via IWYU | expand |
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>
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/.
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
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
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>
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(-)