linux-kernel - Re: [PATCH] video/hdmi: Reorder fields in 'struct hdmi_avi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b110f9e9-f654-6a43-fa23-ea461d0bb405@gmx.de>
Date:   Mon, 19 Jun 2023 08:11:58 +0200
From:   Helge Deller <deller@....de>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] video/hdmi: Reorder fields in 'struct hdmi_avi_infoframe'

On 6/18/23 22:52, Christophe JAILLET wrote:
> Group some variables based on their sizes to reduce hole and avoid padding.
> On x86_64, this shrinks the size of 'struct hdmi_avi_infoframe'
> from 68 to 60 bytes.
>
> It saves a few bytes of memory and is more cache-line friendly.
>
> This also reduces the union hdmi_infoframe the same way.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>

applied.

Thanks!
Helge


> ---
> Using pahole
>
> Before:
> ======
> struct hdmi_avi_infoframe {
> 	enum hdmi_infoframe_type   type;                 /*     0     4 */
> 	unsigned char              version;              /*     4     1 */
> 	unsigned char              length;               /*     5     1 */
>
> 	/* XXX 2 bytes hole, try to pack */
>
> 	enum hdmi_colorspace       colorspace;           /*     8     4 */
> 	enum hdmi_scan_mode        scan_mode;            /*    12     4 */
> 	enum hdmi_colorimetry      colorimetry;          /*    16     4 */
> 	enum hdmi_picture_aspect   picture_aspect;       /*    20     4 */
> 	enum hdmi_active_aspect    active_aspect;        /*    24     4 */
> 	bool                       itc;                  /*    28     1 */
>
> 	/* XXX 3 bytes hole, try to pack */
>
> 	enum hdmi_extended_colorimetry extended_colorimetry; /*    32     4 */
> 	enum hdmi_quantization_range quantization_range; /*    36     4 */
> 	enum hdmi_nups             nups;                 /*    40     4 */
> 	unsigned char              video_code;           /*    44     1 */
>
> 	/* XXX 3 bytes hole, try to pack */
>
> 	enum hdmi_ycc_quantization_range ycc_quantization_range; /*    48     4 */
> 	enum hdmi_content_type     content_type;         /*    52     4 */
> 	unsigned char              pixel_repeat;         /*    56     1 */
>
> 	/* XXX 1 byte hole, try to pack */
>
> 	short unsigned int         top_bar;              /*    58     2 */
> 	short unsigned int         bottom_bar;           /*    60     2 */
> 	short unsigned int         left_bar;             /*    62     2 */
> 	/* --- cacheline 1 boundary (64 bytes) --- */
> 	short unsigned int         right_bar;            /*    64     2 */
>
> 	/* size: 68, cachelines: 2, members: 20 */
> 	/* sum members: 57, holes: 4, sum holes: 9 */
> 	/* padding: 2 */
> 	/* last cacheline: 4 bytes */
> };
>
>
> After:
> =====
> struct hdmi_avi_infoframe {
> 	enum hdmi_infoframe_type   type;                 /*     0     4 */
> 	unsigned char              version;              /*     4     1 */
> 	unsigned char              length;               /*     5     1 */
> 	bool                       itc;                  /*     6     1 */
> 	unsigned char              pixel_repeat;         /*     7     1 */
> 	enum hdmi_colorspace       colorspace;           /*     8     4 */
> 	enum hdmi_scan_mode        scan_mode;            /*    12     4 */
> 	enum hdmi_colorimetry      colorimetry;          /*    16     4 */
> 	enum hdmi_picture_aspect   picture_aspect;       /*    20     4 */
> 	enum hdmi_active_aspect    active_aspect;        /*    24     4 */
> 	enum hdmi_extended_colorimetry extended_colorimetry; /*    28     4 */
> 	enum hdmi_quantization_range quantization_range; /*    32     4 */
> 	enum hdmi_nups             nups;                 /*    36     4 */
> 	unsigned char              video_code;           /*    40     1 */
>
> 	/* XXX 3 bytes hole, try to pack */
>
> 	enum hdmi_ycc_quantization_range ycc_quantization_range; /*    44     4 */
> 	enum hdmi_content_type     content_type;         /*    48     4 */
> 	short unsigned int         top_bar;              /*    52     2 */
> 	short unsigned int         bottom_bar;           /*    54     2 */
> 	short unsigned int         left_bar;             /*    56     2 */
> 	short unsigned int         right_bar;            /*    58     2 */
>
> 	/* size: 60, cachelines: 1, members: 20 */
> 	/* sum members: 57, holes: 1, sum holes: 3 */
> 	/* last cacheline: 60 bytes */
> };
> ---
>   include/linux/hdmi.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 2f4dcc8d060e..3bb87bf6bc65 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -170,19 +170,19 @@ struct hdmi_avi_infoframe {
>   	enum hdmi_infoframe_type type;
>   	unsigned char version;
>   	unsigned char length;
> +	bool itc;
> +	unsigned char pixel_repeat;
>   	enum hdmi_colorspace colorspace;
>   	enum hdmi_scan_mode scan_mode;
>   	enum hdmi_colorimetry colorimetry;
>   	enum hdmi_picture_aspect picture_aspect;
>   	enum hdmi_active_aspect active_aspect;
> -	bool itc;
>   	enum hdmi_extended_colorimetry extended_colorimetry;
>   	enum hdmi_quantization_range quantization_range;
>   	enum hdmi_nups nups;
>   	unsigned char video_code;
>   	enum hdmi_ycc_quantization_range ycc_quantization_range;
>   	enum hdmi_content_type content_type;
> -	unsigned char pixel_repeat;
>   	unsigned short top_bar;
>   	unsigned short bottom_bar;
>   	unsigned short left_bar;