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

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

[George Guo]

From: George Guo <guodongtai@kylinos.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.

Signed-off-by: George Guo <guodongtai@kylinos.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);
 
 	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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mark Rutland]

On Mon, Apr 22, 2024 at 02:38:53PM +0800, George Guo wrote:
> From: George Guo <guodongtai@kylinos.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@kylinos.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
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 0/1] cleanup arch_static_branch/_jump

[George Guo]

From: George Guo <guodongtai@kylinos.cn>

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[George Guo]

From: George Guo <guodongtai@kylinos.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.

Signed-off-by: George Guo <guodongtai@kylinos.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);
 
 	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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 0/1] cleanup arch_static_branch/_jump

[George Guo]

From: George Guo <guodongtai@kylinos.cn>

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[George Guo]

From: George Guo <guodongtai@kylinos.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.

Signed-off-by: George Guo <guodongtai@kylinos.cn>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Will Deacon]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel