On Fri, Dec 13, 2019 at 12:06:01AM +0000, Dmitry Safonov wrote:
> At the current place members those follow are:
> : upf_t flags;
> : upstat_t status;
> : int hw_stopped;
> : unsigned int mctrl;
> : unsigned int timeout;
> : unsigned int type;
> : const struct uart_ops *ops;
>
> Together, they give (*ops) 8-byte align on 64-bit platforms.
> And `sysrq_ch` introduces 4-byte padding.
>
> On the other side, above:
> : struct device *dev;
> : unsigned char hub6;
> : unsigned char suspended;
> : unsigned char unused[2];
> : const char *name;
>
> Adds another 4-byte padding.
>
> Moving sysrq members just before `hub6` allows to save 8 bytes
> per-uart_port on 64-bit platforms:
> On my gcc, x86_64 sizeof(struct uart_port) goes from 528 to 520.
>
> Signed-off-by: Dmitry Safonov <dima@xxxxxxxxxx>
> ---
> include/linux/serial_core.h | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 2b78cc734719..bbbe57bf5163 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -161,11 +161,6 @@ struct uart_port {
> struct uart_icount icount; /* statistics */
>
> struct console *cons; /* struct console, if any */
> -#if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(SUPPORT_SYSRQ)
> - unsigned long sysrq; /* sysrq timeout */
> - unsigned int sysrq_ch; /* char for sysrq */
> -#endif
> -
> /* flags must be updated while holding port mutex */
> upf_t flags;
>
> @@ -244,6 +239,12 @@ struct uart_port {
> resource_size_t mapbase; /* for ioremap */
> resource_size_t mapsize;
> struct device *dev; /* parent device */
> +
> +#if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(SUPPORT_SYSRQ)
> + unsigned long sysrq; /* sysrq timeout */
> + unsigned int sysrq_ch; /* char for sysrq */
> +#endif
Let's just always have these 2 fields in the structure, no need for a
#ifdef at all, right? There is no real "savings" from removing them (8
bytes for a structure that is maybe created 2-4 times per machine?)
That way it would keep any of this mess of if the SUPPORT_SYSRQ is
enabled or not to keep everything in sync.
thanks,
greg k-h
