2023-12-26 18:00:58

by Tanzir Hasan

[permalink] [raw]
Subject: [PATCH v6 0/2] shrink lib/string.i via IWYU

This patch series changes the include list of string.c to minimize
the preprocessing size. The patch series intends to remove REPEAT_BYE
from kernel.h and move it into its own header file because
word-at-a-time.h has an implicit dependancy on it but it is declared
in kernel.h which is bloated.

---

---
Changes in v6:
- added linux/bitops.h to wordpart.h
- removed kernel.h from x86
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Removed bad copyright.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- Fixed personal email client so name appears instead of just email
- Removed kernel.h where not needed.
- Sorted include list in lib/string.c and used linux/limits.h
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Moved REPEAT_BYTE out of kernel.h and into wordpart.h.
- Included wordpart.h where REPEAT_BYTE was necessary.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Transformed into a patch series
- Changed asm inclusions to linux inclusions
- added a patch to sh
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Tanzir Hasan (2):
kernel.h: removed REPEAT_BYTE from kernel.h
lib/string: shrink lib/string.i via IWYU

arch/arm/include/asm/word-at-a-time.h | 3 ++-
arch/arm64/include/asm/word-at-a-time.h | 3 ++-
arch/powerpc/include/asm/word-at-a-time.h | 4 ++--
arch/riscv/include/asm/word-at-a-time.h | 3 ++-
arch/s390/include/asm/word-at-a-time.h | 3 ++-
arch/sh/include/asm/word-at-a-time.h | 2 ++
arch/x86/include/asm/word-at-a-time.h | 3 ++-
arch/x86/kvm/mmu/mmu.c | 1 +
fs/namei.c | 2 +-
include/asm-generic/word-at-a-time.h | 3 ++-
include/linux/kernel.h | 7 -------
include/linux/wordpart.h | 14 ++++++++++++++
lib/string.c | 17 +++++++++--------
13 files changed, 41 insertions(+), 24 deletions(-)
---
base-commit: ceb6a6f023fd3e8b07761ed900352ef574010bcb
change-id: 20231204-libstringheader-e238e2af5eec

Best regards,
--
Tanzir Hasan <[email protected]>



2023-12-26 18:01:13

by Tanzir Hasan

[permalink] [raw]
Subject: [PATCH v6 1/2] kernel.h: removed REPEAT_BYTE from kernel.h

This patch creates wordpart.h and includes it in asm/word-at-a-time.h
for all architectures. WORD_AT_A_TIME_CONSTANTS depends on kernel.h
because of REPEAT_BYTE. Moving this to another header and including it
where necessary allows us to not include the bloated kernel.h. Making
this implicit dependency on REPEAT_BYTE explicit allows for later
improvements in the lib/string.c inclusion list.

Suggested-by: Al Viro <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Tanzir Hasan <[email protected]>
---
arch/arm/include/asm/word-at-a-time.h | 3 ++-
arch/arm64/include/asm/word-at-a-time.h | 3 ++-
arch/powerpc/include/asm/word-at-a-time.h | 4 ++--
arch/riscv/include/asm/word-at-a-time.h | 3 ++-
arch/s390/include/asm/word-at-a-time.h | 3 ++-
arch/sh/include/asm/word-at-a-time.h | 2 ++
arch/x86/include/asm/word-at-a-time.h | 3 ++-
arch/x86/kvm/mmu/mmu.c | 1 +
fs/namei.c | 2 +-
include/asm-generic/word-at-a-time.h | 3 ++-
include/linux/kernel.h | 7 -------
include/linux/wordpart.h | 14 ++++++++++++++
12 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/arch/arm/include/asm/word-at-a-time.h b/arch/arm/include/asm/word-at-a-time.h
index 352ab213520d..f9a3897b06e7 100644
--- a/arch/arm/include/asm/word-at-a-time.h
+++ b/arch/arm/include/asm/word-at-a-time.h
@@ -8,7 +8,8 @@
* Little-endian word-at-a-time zero byte handling.
* Heavily based on the x86 algorithm.
*/
-#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/wordpart.h>

struct word_at_a_time {
const unsigned long one_bits, high_bits;
diff --git a/arch/arm64/include/asm/word-at-a-time.h b/arch/arm64/include/asm/word-at-a-time.h
index f3b151ed0d7a..14251abee23c 100644
--- a/arch/arm64/include/asm/word-at-a-time.h
+++ b/arch/arm64/include/asm/word-at-a-time.h
@@ -9,7 +9,8 @@

#ifndef __AARCH64EB__

-#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/wordpart.h>

struct word_at_a_time {
const unsigned long one_bits, high_bits;
diff --git a/arch/powerpc/include/asm/word-at-a-time.h b/arch/powerpc/include/asm/word-at-a-time.h
index 30a12d208687..54653a863414 100644
--- a/arch/powerpc/include/asm/word-at-a-time.h
+++ b/arch/powerpc/include/asm/word-at-a-time.h
@@ -4,8 +4,8 @@
/*
* Word-at-a-time interfaces for PowerPC.
*/
-
-#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/wordpart.h>
#include <asm/asm-compat.h>
#include <asm/extable.h>

diff --git a/arch/riscv/include/asm/word-at-a-time.h b/arch/riscv/include/asm/word-at-a-time.h
index 7c086ac6ecd4..fae33cc04f8d 100644
--- a/arch/riscv/include/asm/word-at-a-time.h
+++ b/arch/riscv/include/asm/word-at-a-time.h
@@ -9,7 +9,8 @@
#define _ASM_RISCV_WORD_AT_A_TIME_H


-#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/wordpart.h>

struct word_at_a_time {
const unsigned long one_bits, high_bits;
diff --git a/arch/s390/include/asm/word-at-a-time.h b/arch/s390/include/asm/word-at-a-time.h
index 2579f1694b82..203acd6e431b 100644
--- a/arch/s390/include/asm/word-at-a-time.h
+++ b/arch/s390/include/asm/word-at-a-time.h
@@ -2,7 +2,8 @@
#ifndef _ASM_WORD_AT_A_TIME_H
#define _ASM_WORD_AT_A_TIME_H

-#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/wordpart.h>
#include <asm/asm-extable.h>
#include <asm/bitsperlong.h>

diff --git a/arch/sh/include/asm/word-at-a-time.h b/arch/sh/include/asm/word-at-a-time.h
index 4aa398455b94..95100ce128d6 100644
--- a/arch/sh/include/asm/word-at-a-time.h
+++ b/arch/sh/include/asm/word-at-a-time.h
@@ -5,6 +5,8 @@
#ifdef CONFIG_CPU_BIG_ENDIAN
# include <asm-generic/word-at-a-time.h>
#else
+#include <linux/bitops.h>
+#include <linux/wordpart.h>
/*
* Little-endian version cribbed from x86.
*/
diff --git a/arch/x86/include/asm/word-at-a-time.h b/arch/x86/include/asm/word-at-a-time.h
index 46b4f1f7f354..e8d7d4941c4c 100644
--- a/arch/x86/include/asm/word-at-a-time.h
+++ b/arch/x86/include/asm/word-at-a-time.h
@@ -2,7 +2,8 @@
#ifndef _ASM_WORD_AT_A_TIME_H
#define _ASM_WORD_AT_A_TIME_H

-#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/wordpart.h>

/*
* This is largely generic for little-endian machines, but the
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c57e181bba21..75d0f6b11a48 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -47,6 +47,7 @@
#include <linux/kern_levels.h>
#include <linux/kstrtox.h>
#include <linux/kthread.h>
+#include <linux/wordpart.h>

#include <asm/page.h>
#include <asm/memtype.h>
diff --git a/fs/namei.c b/fs/namei.c
index 71c13b2990b4..189ea53ad635 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -17,8 +17,8 @@

#include <linux/init.h>
#include <linux/export.h>
-#include <linux/kernel.h>
#include <linux/slab.h>
+#include <linux/wordpart.h>
#include <linux/fs.h>
#include <linux/filelock.h>
#include <linux/namei.h>
diff --git a/include/asm-generic/word-at-a-time.h b/include/asm-generic/word-at-a-time.h
index 95a1d214108a..ef3f841c6625 100644
--- a/include/asm-generic/word-at-a-time.h
+++ b/include/asm-generic/word-at-a-time.h
@@ -2,7 +2,8 @@
#ifndef _ASM_WORD_AT_A_TIME_H
#define _ASM_WORD_AT_A_TIME_H

-#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/wordpart.h>
#include <asm/byteorder.h>

#ifdef __BIG_ENDIAN
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d9ad21058eed..162660af5b7d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -39,13 +39,6 @@

#define STACK_MAGIC 0xdeadbeef

-/**
- * REPEAT_BYTE - repeat the value @x multiple times as an unsigned long value
- * @x: value to repeat
- *
- * NOTE: @x is not checked for > 0xff; larger values produce odd results.
- */
-#define REPEAT_BYTE(x) ((~0ul / 0xff) * (x))

/* generic data direction definitions */
#define READ 0
diff --git a/include/linux/wordpart.h b/include/linux/wordpart.h
new file mode 100644
index 000000000000..43c1a8ae5152
--- /dev/null
+++ b/include/linux/wordpart.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_WORDPART_H
+#define _LINUX_WORDPART_H
+/**
+ * REPEAT_BYTE - repeat the value @x multiple times as an unsigned long value
+ * @x: value to repeat
+ *
+ * NOTE: @x is not checked for > 0xff; larger values produce odd results.
+ */
+#define REPEAT_BYTE(x) ((~0ul / 0xff) * (x))
+
+#endif // _LINUX_WORDPART_H
+

--
2.43.0.472.g3155946c3a-goog


2023-12-26 18:01:25

by Tanzir Hasan

[permalink] [raw]
Subject: [PATCH v6 2/2] lib/string: shrink lib/string.i via IWYU

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 5321 lines (-80%) for the x86
defconfig.

Link: https://github.com/ClangBuiltLinux/IWYUScripts
Reviewed-by: Kees Cook <[email protected]>
Signed-off-by: Tanzir Hasan <[email protected]>
---
lib/string.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index be26623953d2..06d9b46875ef 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -15,19 +15,20 @@
*/

#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/ctype.h>
#include <linux/errno.h>
-#include <linux/slab.h>
+#include <linux/limits.h>
+#include <linux/linkage.h>
+#include <linux/stddef.h>
+#include <linux/string.h>
+#include <linux/types.h>

+#include <asm/page.h>
+#include <asm/rwonce.h>
#include <asm/unaligned.h>
-#include <asm/byteorder.h>
#include <asm/word-at-a-time.h>
-#include <asm/page.h>

#ifndef __HAVE_ARCH_STRNCASECMP
/**

--
2.43.0.472.g3155946c3a-goog


2023-12-26 18:35:16

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] lib/string: shrink lib/string.i via IWYU

On Tue, Dec 26, 2023 at 8:00 PM Tanzir Hasan <[email protected]> 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 5321 lines (-80%) for the x86
> defconfig.

Why did you ignore my tag?


--
With Best Regards,
Andy Shevchenko

2023-12-26 18:47:29

by Tanzir Hasan

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] lib/string: shrink lib/string.i via IWYU

On Tue, Dec 26, 2023 at 10:35 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, Dec 26, 2023 at 8:00 PM Tanzir Hasan <[email protected]> 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 5321 lines (-80%) for the x86
> > defconfig.
>
> Why did you ignore my tag?

I'm sorry. I forgot you had already reviewed this one. In a future
revision I will include this:
Reviewed-by: Andy Shevchenko <[email protected]>

Best,
Tanzir

2023-12-27 16:54:23

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] kernel.h: removed REPEAT_BYTE from kernel.h

On Tue, Dec 26, 2023 at 06:00:00PM +0000, Tanzir Hasan wrote:
> This patch creates wordpart.h and includes it in asm/word-at-a-time.h
> for all architectures. WORD_AT_A_TIME_CONSTANTS depends on kernel.h
> because of REPEAT_BYTE. Moving this to another header and including it
> where necessary allows us to not include the bloated kernel.h. Making
> this implicit dependency on REPEAT_BYTE explicit allows for later
> improvements in the lib/string.c inclusion list.

LGTM, and I hope this will not trigger CIs and other build checkers.
Reviewed-by: Andy Shevchenko <[email protected]>

...

> --- a/arch/powerpc/include/asm/word-at-a-time.h
> +++ b/arch/powerpc/include/asm/word-at-a-time.h
> @@ -4,8 +4,8 @@
> /*
> * Word-at-a-time interfaces for PowerPC.
> */

> -

You can preserve this blank line...

> -#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/wordpart.h>

...here.

> #include <asm/asm-compat.h>
> #include <asm/extable.h>

--
With Best Regards,
Andy Shevchenko



2024-02-01 17:52:57

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v6 0/2] shrink lib/string.i via IWYU

On Tue, 26 Dec 2023 17:59:59 +0000, Tanzir Hasan wrote:
> This patch series changes the include list of string.c to minimize
> the preprocessing size. The patch series intends to remove REPEAT_BYE
> from kernel.h and move it into its own header file because
> word-at-a-time.h has an implicit dependancy on it but it is declared
> in kernel.h which is bloated.
>

Applied to for-next/hardening, thanks!

[1/2] kernel.h: removed REPEAT_BYTE from kernel.h
https://git.kernel.org/kees/c/66a5c40f60f5
[2/2] lib/string: shrink lib/string.i via IWYU
https://git.kernel.org/kees/c/38b9baf19469

Take care,

--
Kees Cook