All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 16:23 ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 16:23 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel, Geert Uytterhoeven

Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
large.  It contains fixed-size strings for a name, a type, and a
compatible value, but the first two are barely used.
OF device ID tables contain multiple entries, plus an empty sentinel
entry.

Statistics for my current kernel source tree:
  - 4487 tables with 16836 entries (3367200 bytes)
  - 176 names (average 6.7 max 23 chars)
  - 66 types (average 5.1 max 21 chars)
  - 12192 compatible values (average 18.0 max 45 chars)
Taking into account the minimum needed size to store the strings, only
6.9% of the allocated space is used...

Reduce kernel size by reducing the sizes of the fixed strings by one
half.

This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
KiB.  For a typical kernel supporting a single board, you can expect to
save 50-100 KiB.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Notes:
  - While gcc complains if the non-NUL characters in a string do not fit
    in the available space, it does not complain if there is no space to
    store the string's NUL-terminator.  However, that should be caught
    during testing, as the affected entry won't ever match.  The kernel
    won't crash, as such strings will still be terminated by the
    sentinel in the table.

  - We could save even more by converting the strings from fixed-size
    arrays to pointers, at the expense of making it harder to mark
    entries __init.  Given most drivers support binding and unbinding
    and thus cannot use __init for of_device_id tables, perhaps that's
    the way to go?

Thanks for your comments!
---
 include/linux/mod_devicetable.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index ae2e75d15b219920..2bb2558d52d30d2b 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -266,9 +266,9 @@ struct sdw_device_id {
  * Struct used for matching a device
  */
 struct of_device_id {
-	char	name[32];
-	char	type[32];
-	char	compatible[128];
+	char	name[24];
+	char	type[24];
+	char	compatible[48];
 	const void *data;
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 16:23 ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 16:23 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel, Geert Uytterhoeven

Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
large.  It contains fixed-size strings for a name, a type, and a
compatible value, but the first two are barely used.
OF device ID tables contain multiple entries, plus an empty sentinel
entry.

Statistics for my current kernel source tree:
  - 4487 tables with 16836 entries (3367200 bytes)
  - 176 names (average 6.7 max 23 chars)
  - 66 types (average 5.1 max 21 chars)
  - 12192 compatible values (average 18.0 max 45 chars)
Taking into account the minimum needed size to store the strings, only
6.9% of the allocated space is used...

Reduce kernel size by reducing the sizes of the fixed strings by one
half.

This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
KiB.  For a typical kernel supporting a single board, you can expect to
save 50-100 KiB.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Notes:
  - While gcc complains if the non-NUL characters in a string do not fit
    in the available space, it does not complain if there is no space to
    store the string's NUL-terminator.  However, that should be caught
    during testing, as the affected entry won't ever match.  The kernel
    won't crash, as such strings will still be terminated by the
    sentinel in the table.

  - We could save even more by converting the strings from fixed-size
    arrays to pointers, at the expense of making it harder to mark
    entries __init.  Given most drivers support binding and unbinding
    and thus cannot use __init for of_device_id tables, perhaps that's
    the way to go?

Thanks for your comments!
---
 include/linux/mod_devicetable.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index ae2e75d15b219920..2bb2558d52d30d2b 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -266,9 +266,9 @@ struct sdw_device_id {
  * Struct used for matching a device
  */
 struct of_device_id {
-	char	name[32];
-	char	type[32];
-	char	compatible[128];
+	char	name[24];
+	char	type[24];
+	char	compatible[48];
 	const void *data;
 };
 
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 16:23 ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 16:23 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel, Geert Uytterhoeven

Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
large.  It contains fixed-size strings for a name, a type, and a
compatible value, but the first two are barely used.
OF device ID tables contain multiple entries, plus an empty sentinel
entry.

Statistics for my current kernel source tree:
  - 4487 tables with 16836 entries (3367200 bytes)
  - 176 names (average 6.7 max 23 chars)
  - 66 types (average 5.1 max 21 chars)
  - 12192 compatible values (average 18.0 max 45 chars)
Taking into account the minimum needed size to store the strings, only
6.9% of the allocated space is used...

Reduce kernel size by reducing the sizes of the fixed strings by one
half.

This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
KiB.  For a typical kernel supporting a single board, you can expect to
save 50-100 KiB.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Notes:
  - While gcc complains if the non-NUL characters in a string do not fit
    in the available space, it does not complain if there is no space to
    store the string's NUL-terminator.  However, that should be caught
    during testing, as the affected entry won't ever match.  The kernel
    won't crash, as such strings will still be terminated by the
    sentinel in the table.

  - We could save even more by converting the strings from fixed-size
    arrays to pointers, at the expense of making it harder to mark
    entries __init.  Given most drivers support binding and unbinding
    and thus cannot use __init for of_device_id tables, perhaps that's
    the way to go?

Thanks for your comments!
---
 include/linux/mod_devicetable.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index ae2e75d15b219920..2bb2558d52d30d2b 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -266,9 +266,9 @@ struct sdw_device_id {
  * Struct used for matching a device
  */
 struct of_device_id {
-	char	name[32];
-	char	type[32];
-	char	compatible[128];
+	char	name[24];
+	char	type[24];
+	char	compatible[48];
 	const void *data;
 };
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 16:23 ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 16:23 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: devicetree, Geert Uytterhoeven, linux-mips, linux-kernel,
	linux-renesas-soc, linux-riscv, linuxppc-dev, linux-arm-kernel

Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
large.  It contains fixed-size strings for a name, a type, and a
compatible value, but the first two are barely used.
OF device ID tables contain multiple entries, plus an empty sentinel
entry.

Statistics for my current kernel source tree:
  - 4487 tables with 16836 entries (3367200 bytes)
  - 176 names (average 6.7 max 23 chars)
  - 66 types (average 5.1 max 21 chars)
  - 12192 compatible values (average 18.0 max 45 chars)
Taking into account the minimum needed size to store the strings, only
6.9% of the allocated space is used...

Reduce kernel size by reducing the sizes of the fixed strings by one
half.

This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
KiB.  For a typical kernel supporting a single board, you can expect to
save 50-100 KiB.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Notes:
  - While gcc complains if the non-NUL characters in a string do not fit
    in the available space, it does not complain if there is no space to
    store the string's NUL-terminator.  However, that should be caught
    during testing, as the affected entry won't ever match.  The kernel
    won't crash, as such strings will still be terminated by the
    sentinel in the table.

  - We could save even more by converting the strings from fixed-size
    arrays to pointers, at the expense of making it harder to mark
    entries __init.  Given most drivers support binding and unbinding
    and thus cannot use __init for of_device_id tables, perhaps that's
    the way to go?

Thanks for your comments!
---
 include/linux/mod_devicetable.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index ae2e75d15b219920..2bb2558d52d30d2b 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -266,9 +266,9 @@ struct sdw_device_id {
  * Struct used for matching a device
  */
 struct of_device_id {
-	char	name[32];
-	char	type[32];
-	char	compatible[128];
+	char	name[24];
+	char	type[24];
+	char	compatible[48];
 	const void *data;
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
  2021-11-10 16:23 ` Geert Uytterhoeven
  (?)
  (?)
@ 2021-11-10 16:51   ` Rasmus Villemoes
  -1 siblings, 0 replies; 16+ messages in thread
From: Rasmus Villemoes @ 2021-11-10 16:51 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Frank Rowand
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel

On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...
> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.

Tried something like this 2.5 years ago:
https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I think that there might be some not-in-tree code that relies on the
existing layout. I considered adding a CONFIG_ knob, either for these
sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
which embedded folks that build the entire distro from source and don't
have any legacy things can turn off, and then get more sensible defaults
all around.

Including fx in the TCP stack where some CVE fix required changing some
parameter, but the kernel itself couldn't ship a sane default because
no-regressions, so userspace had to learn to set yet another sysctl
properly.

Rasmus

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 16:51   ` Rasmus Villemoes
  0 siblings, 0 replies; 16+ messages in thread
From: Rasmus Villemoes @ 2021-11-10 16:51 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Frank Rowand
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel

On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...
> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.

Tried something like this 2.5 years ago:
https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I think that there might be some not-in-tree code that relies on the
existing layout. I considered adding a CONFIG_ knob, either for these
sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
which embedded folks that build the entire distro from source and don't
have any legacy things can turn off, and then get more sensible defaults
all around.

Including fx in the TCP stack where some CVE fix required changing some
parameter, but the kernel itself couldn't ship a sane default because
no-regressions, so userspace had to learn to set yet another sysctl
properly.

Rasmus

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 16:51   ` Rasmus Villemoes
  0 siblings, 0 replies; 16+ messages in thread
From: Rasmus Villemoes @ 2021-11-10 16:51 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Frank Rowand
  Cc: devicetree, linux-mips, linux-kernel, linux-renesas-soc,
	linux-riscv, linuxppc-dev, linux-arm-kernel

On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...
> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.

Tried something like this 2.5 years ago:
https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I think that there might be some not-in-tree code that relies on the
existing layout. I considered adding a CONFIG_ knob, either for these
sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
which embedded folks that build the entire distro from source and don't
have any legacy things can turn off, and then get more sensible defaults
all around.

Including fx in the TCP stack where some CVE fix required changing some
parameter, but the kernel itself couldn't ship a sane default because
no-regressions, so userspace had to learn to set yet another sysctl
properly.

Rasmus

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 16:51   ` Rasmus Villemoes
  0 siblings, 0 replies; 16+ messages in thread
From: Rasmus Villemoes @ 2021-11-10 16:51 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Frank Rowand
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel

On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...
> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.

Tried something like this 2.5 years ago:
https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I think that there might be some not-in-tree code that relies on the
existing layout. I considered adding a CONFIG_ knob, either for these
sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
which embedded folks that build the entire distro from source and don't
have any legacy things can turn off, and then get more sensible defaults
all around.

Including fx in the TCP stack where some CVE fix required changing some
parameter, but the kernel itself couldn't ship a sane default because
no-regressions, so userspace had to learn to set yet another sysctl
properly.

Rasmus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
  2021-11-10 16:51   ` Rasmus Villemoes
  (?)
  (?)
@ 2021-11-10 19:07     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 19:07 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Rob Herring, Frank Rowand,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linuxppc-dev, open list:BROADCOM NVRAM DRIVER,
	linux-riscv, Linux-Renesas, Linux Kernel Mailing List

Hi Rasmus,

On Wed, Nov 10, 2021 at 5:51 PM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
> On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> > Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> > large.  It contains fixed-size strings for a name, a type, and a
> > compatible value, but the first two are barely used.
> > OF device ID tables contain multiple entries, plus an empty sentinel
> > entry.
> >
> > Statistics for my current kernel source tree:
> >   - 4487 tables with 16836 entries (3367200 bytes)
> >   - 176 names (average 6.7 max 23 chars)
> >   - 66 types (average 5.1 max 21 chars)
> >   - 12192 compatible values (average 18.0 max 45 chars)
> > Taking into account the minimum needed size to store the strings, only
> > 6.9% of the allocated space is used...
> >
> > Reduce kernel size by reducing the sizes of the fixed strings by one
> > half.
>
> Tried something like this 2.5 years ago:
> https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I wasn't aware of that.  I reworked some code which used multiple
of_find_compatible_node() calls before, and noticed the end result
had grown a lot due to the sheer size of of_device_id
("[PATCH] soc: renesas: Consolidate product register handling",
 https://lore.kernel.org/all/057721f46c7499de4133135488f0f3da7fb39265.1636570669.git.geert+renesas@glider.be).

> I think that there might be some not-in-tree code that relies on the
> existing layout. I considered adding a CONFIG_ knob, either for these
> sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
> which embedded folks that build the entire distro from source and don't
> have any legacy things can turn off, and then get more sensible defaults
> all around.

Most of that should have been gone since the #ifdef KERNEL was removed
from include/linux/mod_devicetable.h in commit 6543becf26fff612
("mod/file2alias: make modalias generation safe for cross compiling").
Of course you can never know for sure...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 19:07     ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 19:07 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Rob Herring, Frank Rowand,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linuxppc-dev, open list:BROADCOM NVRAM DRIVER,
	linux-riscv, Linux-Renesas, Linux Kernel Mailing List

Hi Rasmus,

On Wed, Nov 10, 2021 at 5:51 PM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
> On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> > Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> > large.  It contains fixed-size strings for a name, a type, and a
> > compatible value, but the first two are barely used.
> > OF device ID tables contain multiple entries, plus an empty sentinel
> > entry.
> >
> > Statistics for my current kernel source tree:
> >   - 4487 tables with 16836 entries (3367200 bytes)
> >   - 176 names (average 6.7 max 23 chars)
> >   - 66 types (average 5.1 max 21 chars)
> >   - 12192 compatible values (average 18.0 max 45 chars)
> > Taking into account the minimum needed size to store the strings, only
> > 6.9% of the allocated space is used...
> >
> > Reduce kernel size by reducing the sizes of the fixed strings by one
> > half.
>
> Tried something like this 2.5 years ago:
> https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I wasn't aware of that.  I reworked some code which used multiple
of_find_compatible_node() calls before, and noticed the end result
had grown a lot due to the sheer size of of_device_id
("[PATCH] soc: renesas: Consolidate product register handling",
 https://lore.kernel.org/all/057721f46c7499de4133135488f0f3da7fb39265.1636570669.git.geert+renesas@glider.be).

> I think that there might be some not-in-tree code that relies on the
> existing layout. I considered adding a CONFIG_ knob, either for these
> sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
> which embedded folks that build the entire distro from source and don't
> have any legacy things can turn off, and then get more sensible defaults
> all around.

Most of that should have been gone since the #ifdef KERNEL was removed
from include/linux/mod_devicetable.h in commit 6543becf26fff612
("mod/file2alias: make modalias generation safe for cross compiling").
Of course you can never know for sure...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 19:07     ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 19:07 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linuxppc-dev, open list:BROADCOM NVRAM DRIVER,
	Linux Kernel Mailing List, Linux-Renesas, Rob Herring,
	linux-riscv, Frank Rowand, Linux ARM

Hi Rasmus,

On Wed, Nov 10, 2021 at 5:51 PM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
> On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> > Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> > large.  It contains fixed-size strings for a name, a type, and a
> > compatible value, but the first two are barely used.
> > OF device ID tables contain multiple entries, plus an empty sentinel
> > entry.
> >
> > Statistics for my current kernel source tree:
> >   - 4487 tables with 16836 entries (3367200 bytes)
> >   - 176 names (average 6.7 max 23 chars)
> >   - 66 types (average 5.1 max 21 chars)
> >   - 12192 compatible values (average 18.0 max 45 chars)
> > Taking into account the minimum needed size to store the strings, only
> > 6.9% of the allocated space is used...
> >
> > Reduce kernel size by reducing the sizes of the fixed strings by one
> > half.
>
> Tried something like this 2.5 years ago:
> https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I wasn't aware of that.  I reworked some code which used multiple
of_find_compatible_node() calls before, and noticed the end result
had grown a lot due to the sheer size of of_device_id
("[PATCH] soc: renesas: Consolidate product register handling",
 https://lore.kernel.org/all/057721f46c7499de4133135488f0f3da7fb39265.1636570669.git.geert+renesas@glider.be).

> I think that there might be some not-in-tree code that relies on the
> existing layout. I considered adding a CONFIG_ knob, either for these
> sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
> which embedded folks that build the entire distro from source and don't
> have any legacy things can turn off, and then get more sensible defaults
> all around.

Most of that should have been gone since the #ifdef KERNEL was removed
from include/linux/mod_devicetable.h in commit 6543becf26fff612
("mod/file2alias: make modalias generation safe for cross compiling").
Of course you can never know for sure...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-10 19:07     ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-11-10 19:07 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Rob Herring, Frank Rowand,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linuxppc-dev, open list:BROADCOM NVRAM DRIVER,
	linux-riscv, Linux-Renesas, Linux Kernel Mailing List

Hi Rasmus,

On Wed, Nov 10, 2021 at 5:51 PM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
> On 10/11/2021 17.23, Geert Uytterhoeven wrote:
> > Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> > large.  It contains fixed-size strings for a name, a type, and a
> > compatible value, but the first two are barely used.
> > OF device ID tables contain multiple entries, plus an empty sentinel
> > entry.
> >
> > Statistics for my current kernel source tree:
> >   - 4487 tables with 16836 entries (3367200 bytes)
> >   - 176 names (average 6.7 max 23 chars)
> >   - 66 types (average 5.1 max 21 chars)
> >   - 12192 compatible values (average 18.0 max 45 chars)
> > Taking into account the minimum needed size to store the strings, only
> > 6.9% of the allocated space is used...
> >
> > Reduce kernel size by reducing the sizes of the fixed strings by one
> > half.
>
> Tried something like this 2.5 years ago:
> https://lore.kernel.org/lkml/20190425203101.9403-1-linux@rasmusvillemoes.dk/

I wasn't aware of that.  I reworked some code which used multiple
of_find_compatible_node() calls before, and noticed the end result
had grown a lot due to the sheer size of of_device_id
("[PATCH] soc: renesas: Consolidate product register handling",
 https://lore.kernel.org/all/057721f46c7499de4133135488f0f3da7fb39265.1636570669.git.geert+renesas@glider.be).

> I think that there might be some not-in-tree code that relies on the
> existing layout. I considered adding a CONFIG_ knob, either for these
> sizes in particular, or more generally a def_bool y "CONFIG_LEGACY"
> which embedded folks that build the entire distro from source and don't
> have any legacy things can turn off, and then get more sensible defaults
> all around.

Most of that should have been gone since the #ifdef KERNEL was removed
from include/linux/mod_devicetable.h in commit 6543becf26fff612
("mod/file2alias: make modalias generation safe for cross compiling").
Of course you can never know for sure...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
  2021-11-10 16:23 ` Geert Uytterhoeven
  (?)
  (?)
@ 2021-11-11  4:06   ` Frank Rowand
  -1 siblings, 0 replies; 16+ messages in thread
From: Frank Rowand @ 2021-11-11  4:06 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel

Hi Geert,

On 11/10/21 11:23 AM, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...

I like the idea of using less memory (and thank you for the above data!),
but I do not like the implementation, which reduces the size (of name at
least - I didn't check each field) to less than what the standard allows.

I have an idea of another way to accomplish the same goal, but I need to
dig a bit to make sure I'm not shooting myself in the foot.

-Frank

> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.
> 
> This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
> KiB.  For a typical kernel supporting a single board, you can expect to
> save 50-100 KiB.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Notes:
>   - While gcc complains if the non-NUL characters in a string do not fit
>     in the available space, it does not complain if there is no space to
>     store the string's NUL-terminator.  However, that should be caught
>     during testing, as the affected entry won't ever match.  The kernel
>     won't crash, as such strings will still be terminated by the
>     sentinel in the table.
> 
>   - We could save even more by converting the strings from fixed-size
>     arrays to pointers, at the expense of making it harder to mark
>     entries __init.  Given most drivers support binding and unbinding
>     and thus cannot use __init for of_device_id tables, perhaps that's
>     the way to go?
> 
> Thanks for your comments!
> ---
>  include/linux/mod_devicetable.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index ae2e75d15b219920..2bb2558d52d30d2b 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -266,9 +266,9 @@ struct sdw_device_id {
>   * Struct used for matching a device
>   */
>  struct of_device_id {
> -	char	name[32];
> -	char	type[32];
> -	char	compatible[128];
> +	char	name[24];
> +	char	type[24];
> +	char	compatible[48];
>  	const void *data;
>  };
>  
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-11  4:06   ` Frank Rowand
  0 siblings, 0 replies; 16+ messages in thread
From: Frank Rowand @ 2021-11-11  4:06 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel

Hi Geert,

On 11/10/21 11:23 AM, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...

I like the idea of using less memory (and thank you for the above data!),
but I do not like the implementation, which reduces the size (of name at
least - I didn't check each field) to less than what the standard allows.

I have an idea of another way to accomplish the same goal, but I need to
dig a bit to make sure I'm not shooting myself in the foot.

-Frank

> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.
> 
> This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
> KiB.  For a typical kernel supporting a single board, you can expect to
> save 50-100 KiB.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Notes:
>   - While gcc complains if the non-NUL characters in a string do not fit
>     in the available space, it does not complain if there is no space to
>     store the string's NUL-terminator.  However, that should be caught
>     during testing, as the affected entry won't ever match.  The kernel
>     won't crash, as such strings will still be terminated by the
>     sentinel in the table.
> 
>   - We could save even more by converting the strings from fixed-size
>     arrays to pointers, at the expense of making it harder to mark
>     entries __init.  Given most drivers support binding and unbinding
>     and thus cannot use __init for of_device_id tables, perhaps that's
>     the way to go?
> 
> Thanks for your comments!
> ---
>  include/linux/mod_devicetable.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index ae2e75d15b219920..2bb2558d52d30d2b 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -266,9 +266,9 @@ struct sdw_device_id {
>   * Struct used for matching a device
>   */
>  struct of_device_id {
> -	char	name[32];
> -	char	type[32];
> -	char	compatible[128];
> +	char	name[24];
> +	char	type[24];
> +	char	compatible[48];
>  	const void *data;
>  };
>  
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-11  4:06   ` Frank Rowand
  0 siblings, 0 replies; 16+ messages in thread
From: Frank Rowand @ 2021-11-11  4:06 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring
  Cc: devicetree, linux-mips, linux-kernel, linux-renesas-soc,
	linux-riscv, linuxppc-dev, linux-arm-kernel

Hi Geert,

On 11/10/21 11:23 AM, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...

I like the idea of using less memory (and thank you for the above data!),
but I do not like the implementation, which reduces the size (of name at
least - I didn't check each field) to less than what the standard allows.

I have an idea of another way to accomplish the same goal, but I need to
dig a bit to make sure I'm not shooting myself in the foot.

-Frank

> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.
> 
> This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
> KiB.  For a typical kernel supporting a single board, you can expect to
> save 50-100 KiB.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Notes:
>   - While gcc complains if the non-NUL characters in a string do not fit
>     in the available space, it does not complain if there is no space to
>     store the string's NUL-terminator.  However, that should be caught
>     during testing, as the affected entry won't ever match.  The kernel
>     won't crash, as such strings will still be terminated by the
>     sentinel in the table.
> 
>   - We could save even more by converting the strings from fixed-size
>     arrays to pointers, at the expense of making it harder to mark
>     entries __init.  Given most drivers support binding and unbinding
>     and thus cannot use __init for of_device_id tables, perhaps that's
>     the way to go?
> 
> Thanks for your comments!
> ---
>  include/linux/mod_devicetable.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index ae2e75d15b219920..2bb2558d52d30d2b 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -266,9 +266,9 @@ struct sdw_device_id {
>   * Struct used for matching a device
>   */
>  struct of_device_id {
> -	char	name[32];
> -	char	type[32];
> -	char	compatible[128];
> +	char	name[24];
> +	char	type[24];
> +	char	compatible[48];
>  	const void *data;
>  };
>  
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] of: Shrink struct of_device_id
@ 2021-11-11  4:06   ` Frank Rowand
  0 siblings, 0 replies; 16+ messages in thread
From: Frank Rowand @ 2021-11-11  4:06 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring
  Cc: devicetree, linux-arm-kernel, linuxppc-dev, linux-mips,
	linux-riscv, linux-renesas-soc, linux-kernel

Hi Geert,

On 11/10/21 11:23 AM, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large.  It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
> 
> Statistics for my current kernel source tree:
>   - 4487 tables with 16836 entries (3367200 bytes)
>   - 176 names (average 6.7 max 23 chars)
>   - 66 types (average 5.1 max 21 chars)
>   - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...

I like the idea of using less memory (and thank you for the above data!),
but I do not like the implementation, which reduces the size (of name at
least - I didn't check each field) to less than what the standard allows.

I have an idea of another way to accomplish the same goal, but I need to
dig a bit to make sure I'm not shooting myself in the foot.

-Frank

> 
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.
> 
> This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
> KiB.  For a typical kernel supporting a single board, you can expect to
> save 50-100 KiB.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Notes:
>   - While gcc complains if the non-NUL characters in a string do not fit
>     in the available space, it does not complain if there is no space to
>     store the string's NUL-terminator.  However, that should be caught
>     during testing, as the affected entry won't ever match.  The kernel
>     won't crash, as such strings will still be terminated by the
>     sentinel in the table.
> 
>   - We could save even more by converting the strings from fixed-size
>     arrays to pointers, at the expense of making it harder to mark
>     entries __init.  Given most drivers support binding and unbinding
>     and thus cannot use __init for of_device_id tables, perhaps that's
>     the way to go?
> 
> Thanks for your comments!
> ---
>  include/linux/mod_devicetable.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index ae2e75d15b219920..2bb2558d52d30d2b 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -266,9 +266,9 @@ struct sdw_device_id {
>   * Struct used for matching a device
>   */
>  struct of_device_id {
> -	char	name[32];
> -	char	type[32];
> -	char	compatible[128];
> +	char	name[24];
> +	char	type[24];
> +	char	compatible[48];
>  	const void *data;
>  };
>  
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-11-11  4:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-10 16:23 [PATCH/RFC] of: Shrink struct of_device_id Geert Uytterhoeven
2021-11-10 16:23 ` Geert Uytterhoeven
2021-11-10 16:23 ` Geert Uytterhoeven
2021-11-10 16:23 ` Geert Uytterhoeven
2021-11-10 16:51 ` Rasmus Villemoes
2021-11-10 16:51   ` Rasmus Villemoes
2021-11-10 16:51   ` Rasmus Villemoes
2021-11-10 16:51   ` Rasmus Villemoes
2021-11-10 19:07   ` Geert Uytterhoeven
2021-11-10 19:07     ` Geert Uytterhoeven
2021-11-10 19:07     ` Geert Uytterhoeven
2021-11-10 19:07     ` Geert Uytterhoeven
2021-11-11  4:06 ` Frank Rowand
2021-11-11  4:06   ` Frank Rowand
2021-11-11  4:06   ` Frank Rowand
2021-11-11  4:06   ` Frank Rowand

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.