2023-12-19 18:48:25

by Tanzir Hasan

[permalink] [raw]
Subject: [PATCH v5 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 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 | 2 +-
arch/arm64/include/asm/word-at-a-time.h | 2 +-
arch/powerpc/include/asm/word-at-a-time.h | 2 +-
arch/riscv/include/asm/word-at-a-time.h | 2 +-
arch/s390/include/asm/word-at-a-time.h | 2 +-
arch/sh/include/asm/word-at-a-time.h | 1 +
arch/x86/include/asm/word-at-a-time.h | 1 +
fs/namei.c | 2 +-
include/asm-generic/word-at-a-time.h | 2 +-
include/linux/kernel.h | 7 -------
include/linux/wordpart.h | 14 ++++++++++++++
lib/string.c | 17 +++++++++--------
12 files changed, 32 insertions(+), 22 deletions(-)
---
base-commit: ceb6a6f023fd3e8b07761ed900352ef574010bcb
change-id: 20231204-libstringheader-e238e2af5eec

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



2023-12-19 18:48:54

by Tanzir Hasan

[permalink] [raw]
Subject: [PATCH v5 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-19 18:49:02

by Tanzir Hasan

[permalink] [raw]
Subject: [PATCH v5 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 the 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 | 2 +-
arch/arm64/include/asm/word-at-a-time.h | 2 +-
arch/powerpc/include/asm/word-at-a-time.h | 2 +-
arch/riscv/include/asm/word-at-a-time.h | 2 +-
arch/s390/include/asm/word-at-a-time.h | 2 +-
arch/sh/include/asm/word-at-a-time.h | 1 +
arch/x86/include/asm/word-at-a-time.h | 1 +
fs/namei.c | 2 +-
include/asm-generic/word-at-a-time.h | 2 +-
include/linux/kernel.h | 7 -------
include/linux/wordpart.h | 14 ++++++++++++++
11 files changed, 23 insertions(+), 14 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..e08304996121 100644
--- a/arch/arm/include/asm/word-at-a-time.h
+++ b/arch/arm/include/asm/word-at-a-time.h
@@ -8,7 +8,7 @@
* Little-endian word-at-a-time zero byte handling.
* Heavily based on the x86 algorithm.
*/
-#include <linux/kernel.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..bd8cfbc2b9c3 100644
--- a/arch/arm64/include/asm/word-at-a-time.h
+++ b/arch/arm64/include/asm/word-at-a-time.h
@@ -9,7 +9,7 @@

#ifndef __AARCH64EB__

-#include <linux/kernel.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..26e4f718a674 100644
--- a/arch/powerpc/include/asm/word-at-a-time.h
+++ b/arch/powerpc/include/asm/word-at-a-time.h
@@ -5,7 +5,7 @@
* Word-at-a-time interfaces for PowerPC.
*/

-#include <linux/kernel.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..94bec2f7ba53 100644
--- a/arch/riscv/include/asm/word-at-a-time.h
+++ b/arch/riscv/include/asm/word-at-a-time.h
@@ -9,7 +9,7 @@
#define _ASM_RISCV_WORD_AT_A_TIME_H


-#include <linux/kernel.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..55e66d9371d6 100644
--- a/arch/s390/include/asm/word-at-a-time.h
+++ b/arch/s390/include/asm/word-at-a-time.h
@@ -2,7 +2,7 @@
#ifndef _ASM_WORD_AT_A_TIME_H
#define _ASM_WORD_AT_A_TIME_H

-#include <linux/kernel.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..663658cea69a 100644
--- a/arch/sh/include/asm/word-at-a-time.h
+++ b/arch/sh/include/asm/word-at-a-time.h
@@ -5,6 +5,7 @@
#ifdef CONFIG_CPU_BIG_ENDIAN
# include <asm-generic/word-at-a-time.h>
#else
+#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..c002c864a63e 100644
--- a/arch/x86/include/asm/word-at-a-time.h
+++ b/arch/x86/include/asm/word-at-a-time.h
@@ -3,6 +3,7 @@
#define _ASM_WORD_AT_A_TIME_H

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

/*
* This is largely generic for little-endian machines, but the
diff --git a/fs/namei.c b/fs/namei.c
index 71c13b2990b4..03db8ca3f394 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -17,7 +17,7 @@

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

-#include <linux/kernel.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-19 21:52:32

by Andy Shevchenko

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

On Tue, Dec 19, 2023 at 8:48 PM Tanzir Hasan <[email protected]> wrote:
>
> This patch creates wordpart.h and includes it in asm/word-at-a-time.h
> for the 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.

Same comments as per v4 apply here.
According to the Submitting Patches the best time between versions is
~1w. I understand your desire to close this quicker, but the result is
actually the opposite. Take your time and check carefully what others
commented and recommended. With this series it's fine to send a couple
of times per (working) week. It sounds to me balanced between the
complexity of the change and the people's willingness (having time?)
to review.

Otherwise it's a good start!

--
With Best Regards,
Andy Shevchenko

2023-12-19 21:54:11

by Andy Shevchenko

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

On Tue, Dec 19, 2023 at 8:48 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.

This LGTM now,
Reviewed-by: Andy Shevchenko <[email protected]>

--
With Best Regards,
Andy Shevchenko