Re: [PATCH v2] x86/lib: Do not use local symbols with SYM_CODE_START_LOCAL()

From: Jiri Slaby
Date: Fri May 26 2023 - 02:24:22 EST

On 25. 05. 23, 21:39, Nadav Amit wrote:

On May 25, 2023, at 12:05 PM, Dave Hansen <dave.hansen@xxxxxxxxx> wrote:

On 5/25/23 11:42, Nadav Amit wrote:
From: Nadav Amit <namit@xxxxxxxxxx>

When SYM_CODE_START_LOCAL() is used, the symbols are local but need to
be preserved in the object. However, using the ".L" label prefix does
not retain the symbol in the object.

It is beneficial to be able to map instruction pointers back to symbols,
for instance for profiling. Otherwise, there are code addresses that do
not map back to any symbol. Consequently, the ".L" label prefix should
not be used when SYM_CODE_START_LOCAL() is used.

Few symbols, such as .Lbad_put_user_clac and currently have both the
SYM_CODE_START_LOCAL() invocation and the ".L" prefix. This commit
removes the ".L" prefix from these symbols.

No functional change, other then emitting these symbols into the object,
is intended.

Nadav, could you perhaps do a bit of research on how this situation came
to be? Was it an accident or on purpose that these symbols came to be
.L? Then, could you CC the folks who made this change and ask them
directly if they intended to induce the effects that you find undesirable?

Fair enough. I actually thought it is an oversight, but it now seems
intentional (although I am not sure I understand/agree with the reason).

So apparently, for one of the symbols from my v1 (which was already
removed), I see that Borislav Petkov suggested to prepend the “.L” in
order to for them not to be visible [1].

The reason that is given for not making the functions visible is that
these are "functions with very local names”.

I do not think in this tradeoff not exposing local names worth
preventing profilers (and their users) from understanding where a
sample/trace is was taken. If for instance you look at a branch
trace (e.g., using Intel PT) you want to see the symbol to which a
branch goes to.

Borislav, Jiri - do you agree?

Ah, if it makes tools' output harder to follow, I would indeed switch back to emitting these symbols, i.e. remove the .L prefix from them.

That said:

Acked-by: Jiri Slaby <jirislaby@xxxxxxxxxx>

suse labs