gcc-LTO support for the kernel

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

Posted by Jiri Slaby (SUSE) 3 years, 1 month ago

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) = {		\
 		.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,					\
 	}
 
-- 
2.38.1

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

Posted by Josh Poimboeuf 3 years, 1 month ago

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?

-- 
Josh

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

Posted by Peter Zijlstra 3 years, 1 month ago

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?

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

Posted by Andi Kleen 3 years, 1 month ago

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

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

Posted by Peter Zijlstra 3 years, 1 month ago

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.

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

Posted by Josh Poimboeuf 3 years, 1 month ago

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