[rft, PATCH v1 1/4] kernel.h: Move READ/WRITE definitions to <linux/types.h>

[rft, PATCH v1 1/4] kernel.h: Move READ/WRITE definitions to <linux/types.h>

[Andy Shevchenko]

From: Ingo Molnar <mingo@kernel.org>

Headers shouldn't be forced to include <linux/kernel.h> just to
gain these simple constants.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/kernel.h | 4 ----
 include/linux/types.h  | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index fe6efb24d151..bc3e0364970a 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -44,10 +44,6 @@
  */
 #define REPEAT_BYTE(x)	((~0ul / 0xff) * (x))
 
-/* generic data direction definitions */
-#define READ			0
-#define WRITE			1
-
 /**
  * ARRAY_SIZE - get the number of elements in array @arr
  * @arr: array to be sized
diff --git a/include/linux/types.h b/include/linux/types.h
index ea8cf60a8a79..67846bc43d53 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -125,6 +125,10 @@ typedef s64			int64_t;
 typedef u64 sector_t;
 typedef u64 blkcnt_t;
 
+/* generic data direction definitions */
+#define READ			0
+#define WRITE			1
+
 /*
  * The type of an index into the pagecache.
  */
-- 
2.35.1

[rft, PATCH v1 2/4] kernel.h: Split the hexadecimal related helpers to hex.h

[Andy Shevchenko]

For the sake of cleaning up the kernel.h split the hexadecimal
related helpers to own header called 'hex.h'.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/hex.h    | 35 +++++++++++++++++++++++++++++++++++
 include/linux/kernel.h | 29 +----------------------------
 2 files changed, 36 insertions(+), 28 deletions(-)
 create mode 100644 include/linux/hex.h

diff --git a/include/linux/hex.h b/include/linux/hex.h
new file mode 100644
index 000000000000..2618382e5b0c
--- /dev/null
+++ b/include/linux/hex.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_HEX_H
+#define _LINUX_HEX_H
+
+#include <linux/types.h>
+
+extern const char hex_asc[];
+#define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
+#define hex_asc_hi(x)	hex_asc[((x) & 0xf0) >> 4]
+
+static inline char *hex_byte_pack(char *buf, u8 byte)
+{
+	*buf++ = hex_asc_hi(byte);
+	*buf++ = hex_asc_lo(byte);
+	return buf;
+}
+
+extern const char hex_asc_upper[];
+#define hex_asc_upper_lo(x)	hex_asc_upper[((x) & 0x0f)]
+#define hex_asc_upper_hi(x)	hex_asc_upper[((x) & 0xf0) >> 4]
+
+static inline char *hex_byte_pack_upper(char *buf, u8 byte)
+{
+	*buf++ = hex_asc_upper_hi(byte);
+	*buf++ = hex_asc_upper_lo(byte);
+	return buf;
+}
+
+extern int hex_to_bin(unsigned char ch);
+extern int __must_check hex2bin(u8 *dst, const char *src, size_t count);
+extern char *bin2hex(char *dst, const void *src, size_t count);
+
+bool mac_pton(const char *s, u8 *mac);
+
+#endif
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index bc3e0364970a..7a495e94b087 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -20,6 +20,7 @@
 #include <linux/compiler.h>
 #include <linux/container_of.h>
 #include <linux/bitops.h>
+#include <linux/hex.h>
 #include <linux/kstrtox.h>
 #include <linux/log2.h>
 #include <linux/math.h>
@@ -259,34 +260,6 @@ extern enum system_states {
 	SYSTEM_SUSPEND,
 } system_state;
 
-extern const char hex_asc[];
-#define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
-#define hex_asc_hi(x)	hex_asc[((x) & 0xf0) >> 4]
-
-static inline char *hex_byte_pack(char *buf, u8 byte)
-{
-	*buf++ = hex_asc_hi(byte);
-	*buf++ = hex_asc_lo(byte);
-	return buf;
-}
-
-extern const char hex_asc_upper[];
-#define hex_asc_upper_lo(x)	hex_asc_upper[((x) & 0x0f)]
-#define hex_asc_upper_hi(x)	hex_asc_upper[((x) & 0xf0) >> 4]
-
-static inline char *hex_byte_pack_upper(char *buf, u8 byte)
-{
-	*buf++ = hex_asc_upper_hi(byte);
-	*buf++ = hex_asc_upper_lo(byte);
-	return buf;
-}
-
-extern int hex_to_bin(unsigned char ch);
-extern int __must_check hex2bin(u8 *dst, const char *src, size_t count);
-extern char *bin2hex(char *dst, const void *src, size_t count);
-
-bool mac_pton(const char *s, u8 *mac);
-
 /*
  * General tracing related utility functions - trace_printk(),
  * tracing_on/tracing_off and tracing_start()/tracing_stop
-- 
2.35.1

[rft, PATCH v1 3/4] kernel.h: Split out sprintf() and friends

[Andy Shevchenko]

kernel.h is being used as a dump for all kinds of stuff for a long time.
spintf() and friends are used in many drivers without need of the full
kernel.h dependency train with it.

Here is the attempt on cleaning it up by splitting out sprintf() and
frineds.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/kernel.h  | 30 +-----------------------------
 include/linux/sprintf.h | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 29 deletions(-)
 create mode 100644 include/linux/sprintf.h

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 7a495e94b087..7e9612de01b8 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -29,6 +29,7 @@
 #include <linux/panic.h>
 #include <linux/printk.h>
 #include <linux/build_bug.h>
+#include <linux/sprintf.h>
 #include <linux/static_call_types.h>
 #include <linux/instruction_pointer.h>
 #include <asm/byteorder.h>
@@ -199,35 +200,6 @@ static inline void might_fault(void) { }
 
 void do_exit(long error_code) __noreturn;
 
-extern int num_to_str(char *buf, int size,
-		      unsigned long long num, unsigned int width);
-
-/* lib/printf utilities */
-
-extern __printf(2, 3) int sprintf(char *buf, const char * fmt, ...);
-extern __printf(2, 0) int vsprintf(char *buf, const char *, va_list);
-extern __printf(3, 4)
-int snprintf(char *buf, size_t size, const char *fmt, ...);
-extern __printf(3, 0)
-int vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
-extern __printf(3, 4)
-int scnprintf(char *buf, size_t size, const char *fmt, ...);
-extern __printf(3, 0)
-int vscnprintf(char *buf, size_t size, const char *fmt, va_list args);
-extern __printf(2, 3) __malloc
-char *kasprintf(gfp_t gfp, const char *fmt, ...);
-extern __printf(2, 0) __malloc
-char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
-extern __printf(2, 0)
-const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
-
-extern __scanf(2, 3)
-int sscanf(const char *, const char *, ...);
-extern __scanf(2, 0)
-int vsscanf(const char *, const char *, va_list);
-
-extern int no_hash_pointers_enable(char *str);
-
 extern int get_option(char **str, int *pint);
 extern char *get_options(const char *str, int nints, int *ints);
 extern unsigned long long memparse(const char *ptr, char **retptr);
diff --git a/include/linux/sprintf.h b/include/linux/sprintf.h
new file mode 100644
index 000000000000..9cd0282aab3a
--- /dev/null
+++ b/include/linux/sprintf.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KERNEL_PRINTF_H_
+#define _LINUX_KERNEL_PRINTF_H_
+
+#include <linux/types.h>
+
+int num_to_str(char *buf, int size, unsigned long long num, unsigned int width);
+
+__printf(2, 3) int sprintf(char *buf, const char * fmt, ...);
+__printf(2, 0) int vsprintf(char *buf, const char *, va_list);
+__printf(3, 4) int snprintf(char *buf, size_t size, const char *fmt, ...);
+__printf(3, 0) int vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
+__printf(3, 4) int scnprintf(char *buf, size_t size, const char *fmt, ...);
+__printf(3, 0) int vscnprintf(char *buf, size_t size, const char *fmt, va_list args);
+__printf(2, 3) __malloc char *kasprintf(gfp_t gfp, const char *fmt, ...);
+__printf(2, 0) __malloc char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
+__printf(2, 0) const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
+
+__scanf(2, 3) int sscanf(const char *, const char *, ...);
+__scanf(2, 0) int vsscanf(const char *, const char *, va_list);
+
+int no_hash_pointers_enable(char *str);
+
+#endif	/* _LINUX_KERNEL_PRINTF_H */
-- 
2.35.1

[rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Andy Shevchenko]

kernel.h is being used as a dump for all kinds of stuff for a long time.
ARRAY_SIZE() is used in many drivers without need of the full kernel.h
dependency train with it.

Here is the attempt on cleaning it up by splitting out ARRAY_SIZE().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/array_size.h | 13 +++++++++++++
 include/linux/kernel.h     |  8 +-------
 2 files changed, 14 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/array_size.h

diff --git a/include/linux/array_size.h b/include/linux/array_size.h
new file mode 100644
index 000000000000..c9cdba26555f
--- /dev/null
+++ b/include/linux/array_size.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ARRAY_SIZE_H
+#define _LINUX_ARRAY_SIZE_H
+
+#include <linux/compiler.h>
+
+/**
+ * ARRAY_SIZE - get the number of elements in array @arr
+ * @arr: array to be sized
+ */
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
+
+#endif	/* _LINUX_ARRAY_SIZE_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 7e9612de01b8..011eab2b0e93 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -17,7 +17,7 @@
 #include <linux/linkage.h>
 #include <linux/stddef.h>
 #include <linux/types.h>
-#include <linux/compiler.h>
+#include <linux/array_size.h>
 #include <linux/container_of.h>
 #include <linux/bitops.h>
 #include <linux/hex.h>
@@ -46,12 +46,6 @@
  */
 #define REPEAT_BYTE(x)	((~0ul / 0xff) * (x))
 
-/**
- * ARRAY_SIZE - get the number of elements in array @arr
- * @arr: array to be sized
- */
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
-
 #define PTR_IF(cond, ptr)	((cond) ? (ptr) : NULL)
 
 #define u64_to_user_ptr(x) (		\
-- 
2.35.1

Re: [rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Alexey Dobriyan]

> include/linux/array_size.h | 13 +++++++++++++

All of this is pessimisation unless you're removing

	+#include <linux/array_size.h>

from kernel.h which you aren't doing.

container_of.h is just as silly.

kernel.h might need _some_ cleanup (like panic and tainted stuff) which
is rarely used but ARRAY_SIZE()?

Re: [rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Andy Shevchenko]

On Mon, Oct 24, 2022 at 06:44:50PM +0300, Alexey Dobriyan wrote:
> > include/linux/array_size.h | 13 +++++++++++++
> 
> All of this is pessimisation unless you're removing
> 
> 	+#include <linux/array_size.h>
> 
> from kernel.h which you aren't doing.
> 
> container_of.h is just as silly.
> 
> kernel.h might need _some_ cleanup (like panic and tainted stuff) which
> is rarely used but ARRAY_SIZE()?

Are you suggesting to slow down compilation with inclusion of tons of unneeded
stuff in the zillions of drivers?

Or you are talking that we need to abandon most of the headers and combine
everything into kernel.h? I think this is what is silly.


-- 
With Best Regards,
Andy Shevchenko

Re: [rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Andy Shevchenko]

On Mon, Oct 24, 2022 at 07:05:18PM +0300, Andy Shevchenko wrote:
> On Mon, Oct 24, 2022 at 06:44:50PM +0300, Alexey Dobriyan wrote:
> > > include/linux/array_size.h | 13 +++++++++++++
> > 
> > All of this is pessimisation unless you're removing
> > 
> > 	+#include <linux/array_size.h>
> > 
> > from kernel.h which you aren't doing.
> > 
> > container_of.h is just as silly.
> > 
> > kernel.h might need _some_ cleanup (like panic and tainted stuff) which
> > is rarely used but ARRAY_SIZE()?
> 
> Are you suggesting to slow down compilation with inclusion of tons of unneeded
> stuff in the zillions of drivers?
> 
> Or you are talking that we need to abandon most of the headers and combine
> everything into kernel.h? I think this is what is silly.

Btw, your mail broke the reply chain.

-- 
With Best Regards,
Andy Shevchenko

Re: [rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Alexey Dobriyan]

On Mon, Oct 24, 2022 at 07:05:18PM +0300, Andy Shevchenko wrote:
> On Mon, Oct 24, 2022 at 06:44:50PM +0300, Alexey Dobriyan wrote:
> > > include/linux/array_size.h | 13 +++++++++++++
> > 
> > All of this is pessimisation unless you're removing
> > 
> > 	+#include <linux/array_size.h>
> > 
> > from kernel.h which you aren't doing.
> > 
> > container_of.h is just as silly.
> > 
> > kernel.h might need _some_ cleanup (like panic and tainted stuff) which
> > is rarely used but ARRAY_SIZE()?
> 
> Are you suggesting to slow down compilation with inclusion of tons of unneeded
> stuff in the zillions of drivers?
> 
> Or you are talking that we need to abandon most of the headers and combine
> everything into kernel.h? I think this is what is silly.

It hard to escape kernel.h so you will be including it anyway.
Unless you delete, say, kstrtox.h from kernel.h, it is pointless busywork.

Re: [rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Andy Shevchenko]

On Mon, Oct 24, 2022 at 09:14:29PM +0300, Alexey Dobriyan wrote:
> On Mon, Oct 24, 2022 at 07:05:18PM +0300, Andy Shevchenko wrote:
> > On Mon, Oct 24, 2022 at 06:44:50PM +0300, Alexey Dobriyan wrote:
> > > > include/linux/array_size.h | 13 +++++++++++++
> > > 
> > > All of this is pessimisation unless you're removing
> > > 
> > > 	+#include <linux/array_size.h>
> > > 
> > > from kernel.h which you aren't doing.
> > > 
> > > container_of.h is just as silly.
> > > 
> > > kernel.h might need _some_ cleanup (like panic and tainted stuff) which
> > > is rarely used but ARRAY_SIZE()?
> > 
> > Are you suggesting to slow down compilation with inclusion of tons of unneeded
> > stuff in the zillions of drivers?
> > 
> > Or you are talking that we need to abandon most of the headers and combine
> > everything into kernel.h? I think this is what is silly.
> 
> It hard to escape kernel.h so you will be including it anyway.

It's very important to not include it in the headers.

And this split helps to it a lot. We have container_of() or array_size() used
in macros and/or inliners that are usually appears in the headers. And if you
know what dependency hell means, the kernel.h brings to it a huge mess.

So, try to be constructive, okay?

> Unless you delete, say, kstrtox.h from kernel.h, it is pointless busywork.

-- 
With Best Regards,
Andy Shevchenko

Re: [rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Alexey Dobriyan]

On Tue, Oct 25, 2022 at 11:20:10AM +0300, Andy Shevchenko wrote:
> On Mon, Oct 24, 2022 at 09:14:29PM +0300, Alexey Dobriyan wrote:
> > On Mon, Oct 24, 2022 at 07:05:18PM +0300, Andy Shevchenko wrote:
> > > On Mon, Oct 24, 2022 at 06:44:50PM +0300, Alexey Dobriyan wrote:
> > > > > include/linux/array_size.h | 13 +++++++++++++
> > > > 
> > > > All of this is pessimisation unless you're removing
> > > > 
> > > > 	+#include <linux/array_size.h>
> > > > 
> > > > from kernel.h which you aren't doing.
> > > > 
> > > > container_of.h is just as silly.
> > > > 
> > > > kernel.h might need _some_ cleanup (like panic and tainted stuff) which
> > > > is rarely used but ARRAY_SIZE()?
> > > 
> > > Are you suggesting to slow down compilation with inclusion of tons of unneeded
> > > stuff in the zillions of drivers?
> > > 
> > > Or you are talking that we need to abandon most of the headers and combine
> > > everything into kernel.h? I think this is what is silly.
> > 
> > It hard to escape kernel.h so you will be including it anyway.
> 
> It's very important to not include it in the headers.
> 
> And this split helps to it a lot. We have container_of() or array_size() used
> in macros and/or inliners that are usually appears in the headers. And if you
> know what dependency hell means, the kernel.h brings to it a huge mess.
> 
> So, try to be constructive, okay?
> 
> > Unless you delete, say, kstrtox.h from kernel.h, it is pointless busywork.

I'm very constructive.

Parsing 1 define is faster than opening/reading/closing a file to parse
1 define (it's 2 defines because of header guard).

Therefore extracting 1 macro into separate file without obvious future
growth is a pessimisation.

And if you delete #include <array_size.h> from kernel.h and fix all
compile failures (which you aren't doing apparently) backporters will
hate you for life.

I've tried to delete kstrtox.h, and even allnoconfig had dozens of
failures.

Re: [rft, PATCH v1 4/4] kernel.h: Split out ARRAY_SZIE()

[Andy Shevchenko]

On Sat, Oct 29, 2022 at 01:05:43PM +0300, Alexey Dobriyan wrote:
> On Tue, Oct 25, 2022 at 11:20:10AM +0300, Andy Shevchenko wrote:
> > On Mon, Oct 24, 2022 at 09:14:29PM +0300, Alexey Dobriyan wrote:
> > > On Mon, Oct 24, 2022 at 07:05:18PM +0300, Andy Shevchenko wrote:
> > > > On Mon, Oct 24, 2022 at 06:44:50PM +0300, Alexey Dobriyan wrote:
> > > > > > include/linux/array_size.h | 13 +++++++++++++
> > > > > 
> > > > > All of this is pessimisation unless you're removing
> > > > > 
> > > > > 	+#include <linux/array_size.h>
> > > > > 
> > > > > from kernel.h which you aren't doing.
> > > > > 
> > > > > container_of.h is just as silly.
> > > > > 
> > > > > kernel.h might need _some_ cleanup (like panic and tainted stuff) which
> > > > > is rarely used but ARRAY_SIZE()?
> > > > 
> > > > Are you suggesting to slow down compilation with inclusion of tons of unneeded
> > > > stuff in the zillions of drivers?
> > > > 
> > > > Or you are talking that we need to abandon most of the headers and combine
> > > > everything into kernel.h? I think this is what is silly.
> > > 
> > > It hard to escape kernel.h so you will be including it anyway.
> > 
> > It's very important to not include it in the headers.
> > 
> > And this split helps to it a lot. We have container_of() or array_size() used
> > in macros and/or inliners that are usually appears in the headers. And if you
> > know what dependency hell means, the kernel.h brings to it a huge mess.
> > 
> > So, try to be constructive, okay?
> > 
> > > Unless you delete, say, kstrtox.h from kernel.h, it is pointless busywork.
> 
> I'm very constructive.
> 
> Parsing 1 define is faster than opening/reading/closing a file to parse
> 1 define (it's 2 defines because of header guard).
> 
> Therefore extracting 1 macro into separate file without obvious future
> growth is a pessimisation.

You are looking at it at the wrong side. The opening one file inside a single
file maybe a way to slow down the compilation, but cleaning up the _headers_
from kernel.h and similar mess is a definite win.

Ingo, for example, unwounded the thread coil for the scheduler code with a
significant win. And his work exactly shows why it's right way to go.

> And if you delete #include <array_size.h> from kernel.h and fix all
> compile failures (which you aren't doing apparently) backporters will
> hate you for life.

It would need some time to have kernel.h to be hanging as is before we can
start cleaning the mess our of it. I prefer to kill the whole header or leave
there only really _kernel_ parts.

> I've tried to delete kstrtox.h, and even allnoconfig had dozens of
> failures.

So, help us and fix them!

-- 
With Best Regards,
Andy Shevchenko