[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZiYs3itsmdOmuPq4@FVFF77S0Q05N>
Date: Mon, 22 Apr 2024 10:24:46 +0100
From: Mark Rutland <mark.rutland@....com>
To: George Guo <dongtai.guo@...ux.dev>
Cc: peterz@...radead.org, jpoimboe@...nel.org, jbaron@...mai.com,
rostedt@...dmis.org, ardb@...nel.org, catalin.marinas@....com,
will@...nel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, George Guo <guodongtai@...inos.cn>
Subject: Re: [PATCH 1/1] arm64: optimize code duplication in
arch_static_branch/_jump function
On Mon, Apr 22, 2024 at 02:38:53PM +0800, George Guo wrote:
> From: George Guo <guodongtai@...inos.cn>
>
> Extracted the jump table definition code from the arch_static_branch and
> arch_static_branch_jump functions into a macro JUMP_TABLE_ENTRY to reduce
> code duplication and improve readability.
The commit title says this is an optimization, but the commit message says this
is a cleanup (and this clearly is not an optimization).
This seems to be copying what x86 did in commit:
e1aa35c4c4bc71e4 ("jump_label, x86: Factor out the __jump_table generation")
.. where the commit message is much clearer.
>
> Signed-off-by: George Guo <guodongtai@...inos.cn>
> ---
> arch/arm64/include/asm/jump_label.h | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
> index 6aafbb789991..69407b70821e 100644
> --- a/arch/arm64/include/asm/jump_label.h
> +++ b/arch/arm64/include/asm/jump_label.h
> @@ -15,16 +15,19 @@
>
> #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
>
> +#define JUMP_TABLE_ENTRY \
> + ".pushsection __jump_table, \"aw\" \n\t" \
> + ".align 3 \n\t" \
> + ".long 1b - ., %l[l_yes] - . \n\t" \
> + ".quad %c0 - . \n\t" \
> + ".popsection \n\t"
> +
> static __always_inline bool arch_static_branch(struct static_key * const key,
> const bool branch)
> {
> asm goto(
> "1: nop \n\t"
> - " .pushsection __jump_table, \"aw\" \n\t"
> - " .align 3 \n\t"
> - " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> - " .popsection \n\t"
> + JUMP_TABLE_ENTRY
> : : "i"(&((char *)key)[branch]) : : l_yes);
If we really need to factor this out, I'd prefer that the JUMP_TABLE_ENTRY()
macro took the label and key as arguments, similar to what we do for
_ASM_EXTABLE_*().
Mark.
>
> return false;
> @@ -37,11 +40,7 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
> {
> asm goto(
> "1: b %l[l_yes] \n\t"
> - " .pushsection __jump_table, \"aw\" \n\t"
> - " .align 3 \n\t"
> - " .long 1b - ., %l[l_yes] - . \n\t"
> - " .quad %c0 - . \n\t"
> - " .popsection \n\t"
> + JUMP_TABLE_ENTRY
> : : "i"(&((char *)key)[branch]) : : l_yes);
>
> return false;
> --
> 2.34.1
>
>
Powered by blists - more mailing lists