LinuxLists.cc - [PATCH 1/1] arm64: optimize code duplication in arch_static_branch/

2024-04-22 06:39:30

Subject: [PATCH 1/1] arm64: optimize code duplication in arch_static_branch/_jump function

From: George Guo <[email protected]>

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.

Signed-off-by: George Guo <[email protected]>
---
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);

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

2024-04-22 09:30:43

by Mark Rutland

[permalink] [raw]

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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
>

2024-04-25 09:25:45

by George Guo

[permalink] [raw]

Subject: [PATCH v2 0/1] cleanup arch_static_branch/_jump

From: George Guo <[email protected]>

Sorry, this is a cleanup patch.

And I have tried to define macro with key and lable, it seems not clear and
makes things complicated.

So keep the patch and rewrite it's subject.

George Guo (1):
arm64: simplify arch_static_branch/_jump function

arch/arm64/include/asm/jump_label.h | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

--
2.34.1

2024-04-25 09:25:59

by George Guo

[permalink] [raw]

Subject: [PATCH v2 1/1] arm64: simplify arch_static_branch/_jump function

2024-04-30 08:59:07

by George Guo

[permalink] [raw]

Subject: [PATCH v3 0/1] cleanup arch_static_branch/_jump

From: George Guo <[email protected]>

Hi Mark,
I have tried to define macro with key and lable in v3.
What do you think on v3 patch?

George Guo (1):
arm64: simplify arch_static_branch/_jump function

arch/arm64/include/asm/jump_label.h | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)

--
2.34.1

2024-04-30 08:59:17

by George Guo

[permalink] [raw]

Subject: [PATCH v3 1/1] arm64: simplify arch_static_branch/_jump function

From: George Guo <[email protected]>

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.

Signed-off-by: George Guo <[email protected]>
---
arch/arm64/include/asm/jump_label.h | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
index 6aafbb789991..d43fce47feb9 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -15,17 +15,23 @@

#define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE

+#define JUMP_TABLE_ENTRY(key, label) \
+ ".pushsection __jump_table, \"aw\"\n\t" \
+ ".align 3\n\t" \
+ ".long 1b - ., %l["#label"] - .\n\t" \
+ ".quad %c0 - .\n\t" \
+ ".popsection\n\t" \
+ : : "i"(key) : : label
+
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
{
+ char *k = &((char *)key)[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"
- : : "i"(&((char *)key)[branch]) : : l_yes);
+ JUMP_TABLE_ENTRY(k, l_yes)
+ );

return false;
l_yes:
@@ -35,15 +41,11 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
static __always_inline bool arch_static_branch_jump(struct static_key * const key,
const bool branch)
{
+ char *k = &((char *)key)[branch];
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"
- : : "i"(&((char *)key)[branch]) : : l_yes);
-
+ JUMP_TABLE_ENTRY(k, l_yes)
+ );
return false;
l_yes:
return true;
--
2.34.1

2024-05-03 17:33:22

by Will Deacon

[permalink] [raw]

Subject: Re: [PATCH v3 0/1] cleanup arch_static_branch/_jump

On Tue, 30 Apr 2024 16:56:54 +0800, George Guo wrote:
> I have tried to define macro with key and lable in v3.
> What do you think on v3 patch?
>
> George Guo (1):
> arm64: simplify arch_static_branch/_jump function
>
> arch/arm64/include/asm/jump_label.h | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> [...]

Applied to arm64 (for-next/misc), thanks!

[1/1] arm64: simplify arch_static_branch/_jump function
https://git.kernel.org/arm64/c/588de8c6d362

Cheers,
--
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev