mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nadav Amit <nadav.amit@gmail.com>
To: Dave Hansen <dave.hansen@intel.com>,
	Borislav Petkov <bp@alien8.de>, Jiri Slaby <jirislaby@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] x86/lib: Do not use local symbols with SYM_CODE_START_LOCAL()
Date: Thu, 25 May 2023 12:39:47 -0700	[thread overview]
Message-ID: <24E47178-C177-425F-A8EF-CFFAE22597D4@gmail.com> (raw)
In-Reply-To: <38e24fd4-9213-229d-9919-7ae3bfb113bb@intel.com>


> 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?


[1] https://lore.kernel.org/all/20190906075550.23435-2-jslaby@suse.cz/


  reply	other threads:[~2023-05-25 19:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-25 18:42 Nadav Amit
2023-05-25 19:05 ` Dave Hansen
2023-05-25 19:39   ` Nadav Amit [this message]
2023-05-26  6:24     ` Jiri Slaby
2023-05-26 15:53     ` Borislav Petkov
2023-05-26 17:29       ` Nadav Amit
2023-05-26 20:45         ` Borislav Petkov
2023-05-26 21:10           ` Nadav Amit
2023-05-26 21:17             ` Dave Hansen
2023-05-26 21:55               ` Nadav Amit
2023-05-27  7:23                 ` Borislav Petkov
2023-05-27  9:17                   ` Nadav Amit
2023-05-27 12:29                     ` Borislav Petkov
2023-05-27 13:09                       ` Nadav Amit
2023-06-02  0:53                       ` Nadav Amit
2023-06-02  8:59 ` [tip: x86/misc] x86/lib: Make get/put_user() exception handling a visible symbol tip-bot2 for Nadav Amit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24E47178-C177-425F-A8EF-CFFAE22597D4@gmail.com \
    --to=nadav.amit@gmail.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®