Subject: Re: [PATCH/RFC] of: Shrink struct of_device_id
To:     Geert Uytterhoeven <geert+renesas@glider.be>,
        Rob Herring <robh+dt@kernel.org>
Cc:     devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linuxppc-dev@lists.ozlabs.org, linux-mips@vger.kernel.org,
        linux-riscv@lists.infradead.org, linux-renesas-soc@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <ef59d6fd3b2201b912d5eaa7f7a037d8f9adb744.1636561068.git.geert+renesas@glider.be>
From:   Frank Rowand <frowand.list@gmail.com>
Message-ID: <9e20f1b0-3fc4-920a-dff4-1227f72207cf@gmail.com>
Date:   Wed, 10 Nov 2021 23:06:20 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <ef59d6fd3b2201b912d5eaa7f7a037d8f9adb744.1636561068.git.geert+renesas@glider.be>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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;
>  };
>  
>