Messages in this thread | | | Date | Fri, 26 May 2023 08:24:12 +0200 | Subject | Re: [PATCH v2] x86/lib: Do not use local symbols with SYM_CODE_START_LOCAL() | From | Jiri Slaby <> |
| |
On 25. 05. 23, 21:39, Nadav Amit wrote: > >> On May 25, 2023, at 12:05 PM, Dave Hansen <dave.hansen@intel.com> wrote: >> >> On 5/25/23 11:42, Nadav Amit wrote: >>> From: Nadav Amit <namit@vmware.com> >>> >>> 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@kernel.org>
thanks, -- js suse labs
| |