From: Ingo Molnar <mingo@kernel.org>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
hpa@zytor.com, eranian@google.com, linux-kernel@vger.kernel.org,
fweisbec@gmail.com, akpm@linux-foundation.org,
tglx@linutronix.de, linux-tip-commits@vger.kernel.org,
Robert Richter <robert.richter@amd.com>
Subject: Re: [tip:perf/core] perf/x86: Fix USER/KERNEL tagging of samples
Date: Tue, 10 Jul 2012 10:21:04 +0200 [thread overview]
Message-ID: <20120710082104.GA11187@gmail.com> (raw)
In-Reply-To: <1341906848.3462.92.camel@twins>
* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Mon, 2012-07-09 at 20:41 +0200, Ingo Molnar wrote:
> > > +static unsigned long get_segment_base(unsigned int segment)
> > > +{
> > > + struct desc_struct *desc;
> > > + int idx = segment >> 3;
> > > +
> > > + if ((segment & SEGMENT_TI_MASK) == SEGMENT_LDT) {
> > > + if (idx > LDT_ENTRIES)
> > > + return 0;
> > > +
> > > + desc = current->active_mm->context.ldt;
> > > + } else {
> > > + if (idx > GDT_ENTRIES)
> > > + return 0;
> > > +
> > > + desc = __this_cpu_ptr(&gdt_page.gdt[0]);
> > > + }
> > > +
> > > + return get_desc_base(desc + idx);
> >
> > Shouldn't idx be checked against active_mm->context.ldt.size,
> > not LDT_ENTRIES (which is really just an upper limit)?
>
> Ah indeed, fixed that.
Another boundary condition would be when we intentionally
twiddle the GDT: such as during suspend or during BIOS upcalls.
Can we then get a PMU interrupt? If yes then this will probably
result in garbage:
> > > + desc = __this_cpu_ptr(&gdt_page.gdt[0]);
it won't outright crash, we don't ever deallocate our GDT - but
it will return a garbage RIP.
Then there's also all the Xen craziness with segments ...
Both ought to be rare an uninteresting - but then again,
segmented execution is already rare and uninteresting to begin
with.
So, instead of trying to discover all these weird x86 cases -
with little to no testing done after that - I thought that it
might be more future proof to just handle the cases we are
explicitly interested in: flat code, and pounce in some well
defined way in all the other situations by returning the RIP to
an empty __X86_LEGACY_SEGMENTED_CODE() symbol.
That way we will at least give *some* useful information to the
poor segmented code user, if the profile says:
21.32% [kernel] [k] __X86_LEGACY_SEGMENTED_CODE
11.01% [kernel] [k] kallsyms_expand_symbol
8.29% [kernel] [k] vsnprintf
7.37% libc-2.15.so [.] __strcmp_sse42
6.93% perf [.] symbol_filter
4.20% perf [.] kallsyms__parse
3.92% [kernel] [k] format_decode
3.62% [kernel] [k] string.isra.4
3.59% [kernel] [k] memcpy
3.11% [kernel] [k] strnlen
then the user at least knows that there's 21% of overhead in
some sort of segmented x86 code. Or if they *really* want to
resolve that, they can take your patch and add symbol decoding
to user-space and test it all.
KISS and such.
Linus?
Thanks,
Ingo
next prev parent reply other threads:[~2012-07-10 8:21 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-06 6:20 [tip:perf/core] perf/x86: Fix USER/KERNEL tagging of samples tip-bot for Peter Zijlstra
2012-07-06 16:29 ` Linus Torvalds
2012-07-06 18:12 ` Peter Zijlstra
2012-07-06 18:16 ` Linus Torvalds
2012-07-06 18:34 ` Linus Torvalds
2012-07-06 20:48 ` Peter Zijlstra
2012-07-06 20:59 ` Linus Torvalds
2012-07-09 11:23 ` Peter Zijlstra
2012-07-09 17:55 ` Linus Torvalds
2012-07-10 9:02 ` Peter Zijlstra
2012-07-10 9:48 ` Ingo Molnar
2012-07-10 9:50 ` Peter Zijlstra
2012-07-10 9:52 ` Peter Zijlstra
2012-07-10 9:55 ` Ingo Molnar
2012-07-31 17:57 ` [tip:perf/urgent] perf/x86: Fix USER/ KERNEL tagging of samples properly tip-bot for Peter Zijlstra
2012-07-09 18:41 ` [tip:perf/core] perf/x86: Fix USER/KERNEL tagging of samples Ingo Molnar
2012-07-10 7:54 ` Peter Zijlstra
2012-07-10 8:02 ` Ingo Molnar
2012-07-10 8:21 ` Ingo Molnar [this message]
2012-07-10 8:52 ` Peter Zijlstra
2012-07-10 9:48 ` Ingo Molnar
2012-07-31 18:11 ` H. Peter Anvin
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=20120710082104.GA11187@gmail.com \
--to=mingo@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=akpm@linux-foundation.org \
--cc=eranian@google.com \
--cc=fweisbec@gmail.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=robert.richter@amd.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.