Received-SPF: pass (google.com: domain of linux-kernel+bounces-73087-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240215151642.8970-1-petr.pavlu@suse.com>
In-Reply-To: <20240215151642.8970-1-petr.pavlu@suse.com>
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Tue, 20 Feb 2024 22:39:37 +0900
Message-ID: <CAK7LNAS=knGxd9ZRo37CaKTvjcc28bqNasx+XuqbV3S+XV=HtQ@mail.gmail.com>
Subject: Re: [PATCH v2] kbuild: Use -fmin-function-alignment when available
To: Petr Pavlu <petr.pavlu@suse.com>
Cc: nathan@kernel.org, nicolas@fjasle.eu, mark.rutland@arm.com, 
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 16, 2024 at 12:16=E2=80=AFAM Petr Pavlu <petr.pavlu@suse.com> w=
rote:
>
> GCC recently added option -fmin-function-alignment, which should appear
> in GCC 14. Unlike -falign-functions, this option causes all functions to
> be aligned at the specified value, including the cold ones.
>
> Detect availability of -fmin-function-alignment and use it instead of
> -falign-functions when present. Introduce CC_HAS_SANE_FUNCTION_ALIGNMENT
> and make the workarounds for the broken function alignment conditional
> on this setting.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---

[snip]

> index dfb963d2f862..5a6fed4ad3df 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1920,7 +1920,10 @@ EXPORT_SYMBOL(thread_group_exited);
>   *
>   * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D88345#c11
>   */
> -__weak __function_aligned void abort(void)
> +#ifndef CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT
> +__function_aligned
> +#endif
> +__weak void abort(void)
>  {
>         BUG();


__function_aligned is conditionally defined in
include/linux/compiler_types.h, and then it is
conditionally used in kernel/exit.c

This is unreadable.


You may want to move CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT
to include/linux/compiler_types.h, as this is more
aligned with what you did for __cold.


if !defined(CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT) && \
               CONFIG_FUNCTION_ALIGNMENT > 0
#define __function_aligned       __aligned(CONFIG_FUNCTION_ALIGNMENT)
#else
#define __function_aligned
#endif


However, an even more elegant approach is to unify
the two #ifdef blocks because __cold and __function_aligned
are related to each other.


#if defined(CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT) || \
                 (CONFIG_FUNCTION_ALIGNMENT =3D=3D 0)
#define __cold                 __attribute__((__cold__))
#define __function_aligned
#else
#define __cold
#define __function_aligned     __aligned(CONFIG_FUNCTION_ALIGNMENT)
#endif


--
Best Regards
Masahiro Yamada