LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: mingo@kernel.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: Mon, 9 Jul 2012 10:55:49 -0700
Message-ID: <CA+55aFyF-GT-BAjLCvwZPFCCKkkYz0ZznSSOYw4RVYizZhLE0g@mail.gmail.com> (raw)
In-Reply-To: <1341832997.3462.41.camel@twins>

On Mon, Jul 9, 2012 at 4:23 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>
> How about something like the below?
>
> I've also modified perf_instruction_pointer() to account for the VM86
> and IA32 non-zero segment base cases. At least, I tried to do so, I've
> never had the 'pleasure' of poking at this segment descriptor stuff
> before.

The code base calculations look correct to me per se, although I'd put
the VM86 check into code_segment_base(). But whatever.

> Ingo didn't really like doing that though, his suggestion was to kill
> all those IPs by mapping them to a special value (~0UL or so).

Hmm. I don't care much one way or another, but I don't really see the
advantage of doing that.

The expensive part is the checking - since in all normal cases we'll
be using the standard CS and not be in vm86 mode. And you have to do
the checking whether you then map it to ~0ul or do a proper segment
base calculation. So why not just do it right? Your code seems sane
enough to me.

Now "regs" always contains a proper register state, and when the perf
code wants a linear address, perf_instruction_pointer() seems to do
the right translation. Looks good to me, although I didn't check all
the uses.

However, it is worth pointing out that sp/bp have exactly the same
segment base issue. So if you do stack tracing into user mode, you
should really do the same thing for those. And quite frankly, at that
point vm86 mode and the stack segment matters in other ways than just
the base pointer: a 16-bit stack segment acts fundamentally
differently from a 32-bit one. So at that point it may well make much
more sense to take the approach Ingo suggests, and simply not follow
stack frames at all.

Anyway, ACK for that patch from me. It may not solve all the problems,
but it looks ok.

                    Linus

  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-06  6:20 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 [this message]
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
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=CA+55aFyF-GT-BAjLCvwZPFCCKkkYz0ZznSSOYw4RVYizZhLE0g@mail.gmail.com \
    --to=torvalds@linux-foundation.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=mingo@kernel.org \
    --cc=robert.richter@amd.com \
    --cc=tglx@linutronix.de \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git