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

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

[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 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