diff mbox series

[08/46] static_call, lto: Mark static keys as __visible

Message ID 20221114114344.18650-9-jirislaby@kernel.org
State New
Headers
Series gcc-LTO support for the kernel |

Commit Message

Jiri Slaby Nov. 14, 2022, 11:43 a.m. UTC 
  From: Andi Kleen <andi@firstfloor.org>

Symbols referenced from assembler (either directly or e.f. from
DEFINE_STATIC_KEY()) need to be global and visible in gcc LTO because
they could end up in a different object file than the assembler. This
can lead to linker errors without this patch.

So mark static call functions as __visible, namely static keys here.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Martin Liska <mliska@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 include/linux/static_call.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Peter Zijlstra Nov. 14, 2022, 3:51 p.m. UTC | #1 
On Mon, Nov 14, 2022 at 12:43:06PM +0100, Jiri Slaby (SUSE) wrote:
> From: Andi Kleen <andi@firstfloor.org>
> 
> Symbols referenced from assembler (either directly or e.f. from
> DEFINE_STATIC_KEY()) need to be global and visible in gcc LTO because
> they could end up in a different object file than the assembler. This
> can lead to linker errors without this patch.
> 
> So mark static call functions as __visible, namely static keys here.

Why doesn't llvm-lto need this?

Also, why am I getting a random selection of the patchset?
Josh Poimboeuf Nov. 14, 2022, 6:52 p.m. UTC | #2 
On Mon, Nov 14, 2022 at 04:51:07PM +0100, Peter Zijlstra wrote:
> On Mon, Nov 14, 2022 at 12:43:06PM +0100, Jiri Slaby (SUSE) wrote:
> > From: Andi Kleen <andi@firstfloor.org>
> > 
> > Symbols referenced from assembler (either directly or e.f. from
> > DEFINE_STATIC_KEY()) need to be global and visible in gcc LTO because
> > they could end up in a different object file than the assembler. This
> > can lead to linker errors without this patch.
> > 
> > So mark static call functions as __visible, namely static keys here.
> 
> Why doesn't llvm-lto need this?
> 
> Also, why am I getting a random selection of the patchset?

Same, please Cc me on the whole set next time.
Josh Poimboeuf Nov. 14, 2022, 6:57 p.m. UTC | #3 
On Mon, Nov 14, 2022 at 12:43:06PM +0100, Jiri Slaby (SUSE) wrote:
> From: Andi Kleen <andi@firstfloor.org>
> 
> Symbols referenced from assembler (either directly or e.f. from
> DEFINE_STATIC_KEY()) need to be global and visible in gcc LTO because
> they could end up in a different object file than the assembler. This
> can lead to linker errors without this patch.
> 
> So mark static call functions as __visible, namely static keys here.
> 
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> Cc: Jason Baron <jbaron@akamai.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Martin Liska <mliska@suse.cz>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  include/linux/static_call.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/static_call.h b/include/linux/static_call.h
> index df53bed9d71f..e629ab0c4ca3 100644
> --- a/include/linux/static_call.h
> +++ b/include/linux/static_call.h
> @@ -182,7 +182,7 @@ extern long __static_call_return0(void);
>  
>  #define DEFINE_STATIC_CALL(name, _func)					\
>  	DECLARE_STATIC_CALL(name, _func);				\
> -	struct static_call_key STATIC_CALL_KEY(name) = {		\
> +	__visible struct static_call_key STATIC_CALL_KEY(name) = {		\

Why not __visible_on_lto?
Andi Kleen Nov. 14, 2022, 8:34 p.m. UTC | #4 
On Mon, Nov 14, 2022 at 04:51:07PM +0100, Peter Zijlstra wrote:
> On Mon, Nov 14, 2022 at 12:43:06PM +0100, Jiri Slaby (SUSE) wrote:
> > From: Andi Kleen <andi@firstfloor.org>
> > 
> > Symbols referenced from assembler (either directly or e.f. from
> > DEFINE_STATIC_KEY()) need to be global and visible in gcc LTO because
> > they could end up in a different object file than the assembler. This
> > can lead to linker errors without this patch.
> > 
> > So mark static call functions as __visible, namely static keys here.
> 
> Why doesn't llvm-lto need this?

It has an integrated assembler that can feed this information to the LTO
symbol table, while gas cannot do that.

There was some discussion to extend the gcc top level asm syntax to 
express external symbols, but so far it doesn't exist.

> 
> Also, why am I getting a random selection of the patchset?

Me too.

-Andi
Peter Zijlstra Nov. 17, 2022, 8:24 a.m. UTC | #5 
On Mon, Nov 14, 2022 at 12:34:33PM -0800, Andi Kleen wrote:
> On Mon, Nov 14, 2022 at 04:51:07PM +0100, Peter Zijlstra wrote:
> > On Mon, Nov 14, 2022 at 12:43:06PM +0100, Jiri Slaby (SUSE) wrote:
> > > From: Andi Kleen <andi@firstfloor.org>
> > > 
> > > Symbols referenced from assembler (either directly or e.f. from
> > > DEFINE_STATIC_KEY()) need to be global and visible in gcc LTO because
> > > they could end up in a different object file than the assembler. This
> > > can lead to linker errors without this patch.
> > > 
> > > So mark static call functions as __visible, namely static keys here.
> > 
> > Why doesn't llvm-lto need this?
> 
> It has an integrated assembler that can feed this information to the LTO
> symbol table, while gas cannot do that.
> 
> There was some discussion to extend the gcc top level asm syntax to 
> express external symbols, but so far it doesn't exist.

Urgh, that's ugly too. Why does GCC insist on ugly solutions; clang has
shown it can be done sanely, follow.
diff mbox series

Patch

diff --git a/include/linux/static_call.h b/include/linux/static_call.h
index df53bed9d71f..e629ab0c4ca3 100644
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -182,7 +182,7 @@  extern long __static_call_return0(void);
 
 #define DEFINE_STATIC_CALL(name, _func)					\
 	DECLARE_STATIC_CALL(name, _func);				\
-	struct static_call_key STATIC_CALL_KEY(name) = {		\
+	__visible struct static_call_key STATIC_CALL_KEY(name) = {		\
 		.func = _func,						\
 		.type = 1,						\
 	};								\
@@ -190,7 +190,7 @@  extern long __static_call_return0(void);
 
 #define DEFINE_STATIC_CALL_NULL(name, _func)				\
 	DECLARE_STATIC_CALL(name, _func);				\
-	struct static_call_key STATIC_CALL_KEY(name) = {		\
+	__visible struct static_call_key STATIC_CALL_KEY(name) = {	\
 		.func = NULL,						\
 		.type = 1,						\
 	};								\
@@ -198,7 +198,7 @@  extern long __static_call_return0(void);
 
 #define DEFINE_STATIC_CALL_RET0(name, _func)				\
 	DECLARE_STATIC_CALL(name, _func);				\
-	struct static_call_key STATIC_CALL_KEY(name) = {		\
+	__visible struct static_call_key STATIC_CALL_KEY(name) = {		\
 		.func = __static_call_return0,				\
 		.type = 1,						\
 	};								\
@@ -227,14 +227,14 @@  static inline int static_call_init(void) { return 0; }
 
 #define DEFINE_STATIC_CALL(name, _func)					\
 	DECLARE_STATIC_CALL(name, _func);				\
-	struct static_call_key STATIC_CALL_KEY(name) = {		\
+	__visible struct static_call_key STATIC_CALL_KEY(name) = {		\
 		.func = _func,						\
 	};								\
 	ARCH_DEFINE_STATIC_CALL_TRAMP(name, _func)
 
 #define DEFINE_STATIC_CALL_NULL(name, _func)				\
 	DECLARE_STATIC_CALL(name, _func);				\
-	struct static_call_key STATIC_CALL_KEY(name) = {		\
+	__visible struct static_call_key STATIC_CALL_KEY(name) = {	\
 		.func = NULL,						\
 	};								\
 	ARCH_DEFINE_STATIC_CALL_NULL_TRAMP(name)
@@ -288,7 +288,7 @@  static inline long __static_call_return0(void)
 
 #define __DEFINE_STATIC_CALL(name, _func, _func_init)			\
 	DECLARE_STATIC_CALL(name, _func);				\
-	struct static_call_key STATIC_CALL_KEY(name) = {		\
+	__visible struct static_call_key STATIC_CALL_KEY(name) = {	\
 		.func = _func_init,					\
 	}