[18/46] entry, lto: Mark raw_irqentry_exit_cond_resched() as __visible

  From: Andi Kleen <ak@linux.intel.com>

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 raw_irqentry_exit_cond_resched() as __visible.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Martin Liska <mliska@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 kernel/entry/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On Mon, Nov 14 2022 at 12:43, Jiri Slaby wrote:
> Symbols referenced from assembler (either directly or e.f. from

from assembler? I'm not aware that the assembler references anything.

Also what does e.f. mean? Did you want to write e.g.?

> 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

than the assembler? Are we shipping the assembler in an object file?

> can lead to linker errors without this patch.

git grep -i 'this patch' Documentation/process/

> So mark raw_irqentry_exit_cond_resched() as __visible.

And all that tells me what? I know what you want to say, but it's not
there.

  Symbols in different compilation units which are referenced from
  assembly code either directly or indirectly, e.g. from
  DEFINE_STATIC_KEY(), must be marked visible for GCC based LTO builds.

  Add the missing __visible annotation to raw_irqentry_exit_cond_resched().

See?

There is no 'global' because it's obvious that a symbol in a different
compilation unit must be global to be resolvable. It's also obvious that
code in different compilation units ends up in different object files.

So stating that it's a 'must' to have such symbols marked visible is
good enough for an argument because that tells the reader that this is a
mandatory requirement for an GCC based LTO build.

No?

Thanks,

        tglx
  

On Thu, Nov 17, 2022 at 12:30:34AM +0100, Thomas Gleixner wrote:
> On Mon, Nov 14 2022 at 12:43, Jiri Slaby wrote:
> > Symbols referenced from assembler (either directly or e.f. from
> 
> from assembler? I'm not aware that the assembler references anything.
> 
> Also what does e.f. mean? Did you want to write e.g.?
> 
> > 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
> 
> than the assembler? Are we shipping the assembler in an object file?
> 
> > can lead to linker errors without this patch.
> 
> git grep -i 'this patch' Documentation/process/
> 
> > So mark raw_irqentry_exit_cond_resched() as __visible.
> 
> And all that tells me what? I know what you want to say, but it's not
> there.
> 
>   Symbols in different compilation units which are referenced from
>   assembly code either directly or indirectly, e.g. from
>   DEFINE_STATIC_KEY(), must be marked visible for GCC based LTO builds.
> 
>   Add the missing __visible annotation to raw_irqentry_exit_cond_resched().
> 
> See?
> 
> There is no 'global' because it's obvious that a symbol in a different
> compilation unit must be global to be resolvable. It's also obvious that
> code in different compilation units ends up in different object files.
> 
> So stating that it's a 'must' to have such symbols marked visible is
> good enough for an argument because that tells the reader that this is a
> mandatory requirement for an GCC based LTO build.
> 
> No?

I still don't understand any of it -- this symbol is not static (and
thus lives in the global namespace and it's name must not be mangled
lest it breaks ABI), this symbol has it's address taken, so it must not
be eliminated.

WTF does this crazy LTO thing require __visible on it?

The original Changelog babbles something about multiple object files,
which doesn't make sense either, there is only a single object file with
LTO -- that's sort of the whole point. The translation unit output
becomes some intermediate gunk -- to be used as input for the LTO pass,
but it is not an ELF object file.

The linker takes all these intermediate files, does the global
optimization thing and then generates a real ELF object file.

Anyway; I think we can drop all this crazy on the floor again, since per
the 0/n (which I didn't get) there isn't any actual benefit from using
GCC-LTO, so why should we bother with all this ugly.

I would suggest GCC implement this integrated assembler and follow the
clang lead here -- or people who want LTO use clang. GCC is clearly
inferior here.
  

> I still don't understand any of it -- this symbol is not static (and
> thus lives in the global namespace and it's name must not be mangled
> lest it breaks ABI), this symbol has it's address taken, so it must not
> be eliminated.

It's not eliminated, but is still manged because gcc turns it into 
static due to

-fwhole-program. Maybe this could avoided in gcc, but at least that's 
what it does currently.

I believe disabling -fwhole-program would likely avoid it, but it would 
also prevent some code

transformations because gcc would need to assume that every function can 
be called by

someone it doesn't see.

> WTF does this crazy LTO thing require __visible on it?
>
> The original Changelog babbles something about multiple object files,
> which doesn't make sense either, there is only a single object file with
> LTO -- that's sort of the whole point. The translation unit output
> becomes some intermediate gunk -- to be used as input for the LTO pass,
> but it is not an ELF object file.
>
> The linker takes all these intermediate files, does the global
> optimization thing and then generates a real ELF object file.

That would be a single threaded very very slow global compilation. 
Instead gcc WHOPR uses

partitioning to generate smaller units that can be compiled in parallel 
based on their call dependencies,

and these use different object files from the individual assembler 
invocations.

>
> Anyway; I think we can drop all this crazy on the floor again, since per
> the 0/n (which I didn't get) there isn't any actual benefit from using
> GCC-LTO, so why should we bother with all this ugly.

At least in the past it generated smaller kernels for small configurations.

One benefit that wasn't mentioned is doing type and other checks (e.g. 
constant propagation

through inlining) across files.

In general LTO gives the compiler a lot more freedom to optimize code, 
so even if it's not quite there

yet I think it's beneficial to let users play around with it and see if 
they can get benefits.



-Andi
  

On Thu, Nov 17 2022 at 14:07, Andi Kleen wrote:
>> Anyway; I think we can drop all this crazy on the floor again, since per
>> the 0/n (which I didn't get) there isn't any actual benefit from using
>> GCC-LTO, so why should we bother with all this ugly.
>
> At least in the past it generated smaller kernels for small configurations.
>
> One benefit that wasn't mentioned is doing type and other checks (e.g. 
> constant propagation
>
> through inlining) across files.
>
> In general LTO gives the compiler a lot more freedom to optimize code, 
> so even if it's not quite there
>
> yet I think it's beneficial to let users play around with it and see if 
> they can get benefits.

Sure, they can play around with it but that does not require to merge
all this nonsensical ballast for a half thought out compiler.

If they want to do that they can apply the pile of patches as provided
and play around.

If anything useful comes out of that with sensible changelogs and a
sensible argumentation why supporting a half thought out compiler is
required then we can revisit that.

Up to that point this is all considered to be __invisible.

Thanks,

        tglx
  

> Sure, they can play around with it but that does not require to merge
> all this nonsensical ballast for a half thought out compiler.


You are referring to __visible?

TBH I don't understand the problem. In general __visible is useful 
documentation,

so you know something is used from assembler or other strange contexts. 
Doing such things

explicitly marked instead of implicitly hidden and they just happen to 
work by accident

seems cleaner to me.


I can also see the __visible markings being useful for other purposes, 
e.g. static analysis tools or

dynamic instrumentation like the various sanitizers. Everything that is 
referenced outside

the normal code that the compiler sees may need some special handling.


That said I don't see the point of __visible_in_lto either, it should be 
just all __visible.


Similar argument applies to __noreorder, it's also useful documentation.


There are a few real workarounds in the patchkit that are a bit ugly, 
but __visible isn't it.


>
> If they want to do that they can apply the pile of patches as provided
> and play around.


It's very difficult to maintain out of tree, while in tree it's much 
simpler.

I think Linux should support its primary compiler well and not give up 
due to relatively small obstacles.


-Andi
  

On Fri, Nov 18 2022 at 16:50, Andi Kleen wrote:
>> Sure, they can play around with it but that does not require to merge
>> all this nonsensical ballast for a half thought out compiler.
>
> You are referring to __visible?
>
> TBH I don't understand the problem. In general __visible is useful
> documentation, so you know something is used from assembler or other
> strange contexts.  Doing such things explicitly marked instead of
> implicitly hidden and they just happen to work by accident
> seems cleaner to me.

Seems cleaner is really not a technical argument. Visible is completely
useless. Either a symbol is global and therefore reachable from any
point in the final "executable" or it's not. Whether that reference is
in assembly or from a pointer, static key or whatever does not matter at
all. There is no such thing as a 'strange context'.

Nothing works here by accident. A global symbol is a global symbol
whether it's defined or referenced from C or from ASM or from any other
programming language does not matter at all.

> I can also see the __visible markings being useful for other purposes,
> e.g. static analysis tools or dynamic instrumentation like the various
> sanitizers. Everything that is referenced outside the normal code that
> the compiler sees may need some special handling.

All you have is 'may need' and 'I can see'. Where is the actual use case?

>> If they want to do that they can apply the pile of patches as provided
>> and play around.
>
> It's very difficult to maintain out of tree, while in tree it's much 
> simpler.

Sure. Lots of things are simpler to maintain in tree, but that's not an
argument for merging anything.

> I think Linux should support its primary compiler well and not give up 
> due to relatively small obstacles.

It's not an obstacle. It's a fundamental broken model. clang has proven
that it can be done proper, so there is no reason to proliferate the
inferior.

While you might consider gcc to be the primary compiler, that might have
been true a decade ago. A lot of people prefer clang as their primary
compiler simply because its saner and the maintainers behind it are
working with us and not trying to inflict their half baken crap on us to
spare themself the work to do it right.

Thanks,

        tglx
  

Hi,

On 17. 11. 22, 0:30, Thomas Gleixner wrote:
> On Mon, Nov 14 2022 at 12:43, Jiri Slaby wrote:
>> Symbols referenced from assembler (either directly or e.f. from
> 
> from assembler? I'm not aware that the assembler references anything.

"""
Noun assembler

assembler (countable and uncountable, plural assemblers)

1.  (programming, countable) A program that reads source code written in 
assembly language and produces executable machine code, possibly 
together with information needed by linkers, debuggers and other tools.

2. (computer languages, informal, chiefly uncountable) Assembly language.

     I wrote that program in assembler.
""" [1]

I refer in the above to 2. You refer to 1.

In some languages, incl. mine, we don't distinguish between the two. 
It's always assembler. Yet, that might confuse you, even though it's 
correct as you can see above. I can switch to mode 1 (assembler and 
assembly) for sure.

[1] https://en.wiktionary.org/wiki/assembler

> Also what does e.f. mean? Did you want to write e.g.?

Yes, my and my spellchecker's bad.

>> 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
> 
> than the assembler? Are we shipping the assembler in an object file?

Nope, see above.

>> can lead to linker errors without this patch.
> 
> git grep -i 'this patch' Documentation/process/

Sorry, I don't understand, care to elaborate? None of the lines from the 
output seems to match the case here.

>> So mark raw_irqentry_exit_cond_resched() as __visible.
> 
> And all that tells me what? I know what you want to say, but it's not
> there.
> 
>    Symbols in different compilation units which are referenced from
>    assembly code either directly or indirectly, e.g. from
>    DEFINE_STATIC_KEY(), must be marked visible for GCC based LTO builds.
> 
>    Add the missing __visible annotation to raw_irqentry_exit_cond_resched().
> 
> See?
> 
> There is no 'global' because it's obvious that a symbol in a different
> compilation unit must be global to be resolvable. It's also obvious that
> code in different compilation units ends up in different object files.

It's not about different compilation units. It's about different partitions.

> So stating that it's a 'must' to have such symbols marked visible is
> good enough for an argument because that tells the reader that this is a
> mandatory requirement for an GCC based LTO build.

My bad that I failed to explain properly in the commit log. But we are 
working on throwing all this __visible thing away. Agreed, that it's 
ridiculous/absurd.

thanks,