linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robert Richter <robert.richter@amd.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: David Ahern <dsahern@gmail.com>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	<linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH 08/11] perf tool: precise mode requires exclude_guest
Date: Fri, 3 Aug 2012 15:51:17 +0200	[thread overview]
Message-ID: <20120803135117.GC3732@erda.amd.com> (raw)
In-Reply-To: <1343290109.26034.82.camel@twins>

On 26.07.12 10:08:29, Peter Zijlstra wrote:
> On Wed, 2012-07-25 at 23:16 -0600, David Ahern wrote:
> 
> > Peter's patch (see https://lkml.org/lkml/2012/7/9/298) changes kernel 
> > side to require the use of exclude_guest if the precise modifier is 
> > used, returning -EOPNOTSUPP if exclude_guest is not set. This patch goes 
> > after the user experience: Today if a user specifies -e <event>:p all 
> > other modifiers are reset - including exclude_guest. Going forward we 
> > need :p to imply :pH if a user has not specified a GH modifer.
> > 
> > We could do nothing and handle the unsupported error and try setting the 
> > exclude_guest option - like perf handles other new parameters. But 
> > EOPNOTSUPP is not uniquely tied to this error -- e.g., it could be the 
> > BTS is not supported (:pp). Also, we have no easy way to discriminate :p 
> > from :pG or :pGH. It seems to me perf should not silently undo a user 
> > request on the modifier, but inform the user the request is wrong. For 
> > example if a user request -e cycles:pG it should not be silently turned 
> > into :pH.
> > 
> > And then yesterday, Robert stated that none of the exclude_xxxx 
> > modifiers can be set for the AMD if the precise modifier is used, so we 
> > cannot blindly set exclude_guest if precise_ip is set.
> > 
> > So, seems to me perf need's one action for Intel processors and another 
> > for AMD.
> 
> No, we just need to teach the IBS code about SVM enter/exit.

I aggree that this could be emulated in software by enabling/disabling
the event with a guest/host switch. And, even better, we add this for
every pmu in a generic way. E.g. northbridge counter and I guess also
Intel uncore events do not support G/H counting in hardware. Same to
other pmus that could be imaginable in the future like counters for
IOMMUs or other hardware devices.

But, as some pmus are not related to virtualization or other features
they simply do not need to support those attributes, or we want other
defaults, e.g. enable it system wide. Detecting features with syscall
error checking and then falling back to other defaults does not seem
the right approach to me, because it may require several syscalls to
check *combinations* of supported attributes, makes error logging and
detection more difficult due to noisy log messages and because there
is no strict attribute flag checking in current and older kernels.

I better would like to see a pmu feature flag in the same style as
with /proc/cpuinfo, e.g.:

 $ cat /sys/bus/event_source/devices/cpu/flags
 exclude_host exclude_guest

We also need stricter attribute flag checking, esp. of reseved flags
and for unsupported features in some pmus (I already work on some
patches for this). Userland then checks flags and sets up syscalls
according to the reported flags. The goal should be to avoid syscall
errors at all. Thus, we are able to improve dmesg logging in case of
errors, currently we do not see any message if a syscall fails.

And finally, if a feature could be emulated, we could provide this
emulation of an attr flag to all pmus.

Does this make sense?

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center


  reply	other threads:[~2012-08-03 13:51 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-20 23:25 [PATCH 00/11] perf tool: assorted cleanups and bug fixes David Ahern
2012-07-20 23:25 ` [PATCH 01/11] perf tool: add machine id to modules debug message David Ahern
2012-07-25 19:16   ` [tip:perf/core] perf symbols: Add " tip-bot for David Ahern
2012-07-20 23:25 ` [PATCH 02/11] perf kvm: set name for VM process in guest machine David Ahern
2012-07-25 12:12   ` Jiri Olsa
2012-07-25 19:17   ` [tip:perf/core] perf kvm: Set " tip-bot for David Ahern
2012-07-20 23:25 ` [PATCH 03/11] perf kvm: guest userspace samples should not be lumped with host uspace David Ahern
2012-07-25 12:13   ` Jiri Olsa
2012-07-25 19:18   ` [tip:perf/core] perf kvm: Guest " tip-bot for David Ahern
2012-07-20 23:25 ` [PATCH 04/11] perf kvm: fix bug resolving guest kernel syms - v2 David Ahern
2012-07-25 12:11   ` Jiri Olsa
2012-07-25 19:19   ` [tip:perf/core] perf kvm: Fix bug resolving guest kernel syms tip-bot for David Ahern
2012-07-20 23:25 ` [PATCH 05/11] perf kvm: use strtol for walking guestmount directory David Ahern
2012-07-23 18:09   ` Arnaldo Carvalho de Melo
2012-07-20 23:25 ` [PATCH 06/11] perf kvm: limit repetitive guestmount message to once per directory David Ahern
2012-07-23 18:11   ` Arnaldo Carvalho de Melo
2012-07-25 19:20   ` [tip:perf/core] perf kvm: Limit " tip-bot for David Ahern
2012-07-20 23:25 ` [PATCH 07/11] perf tools: dump exclude_{guest,host}, precise_ip header info too David Ahern
2012-07-25 19:20   ` [tip:perf/core] perf tools: Dump " tip-bot for David Ahern
2012-07-20 23:25 ` [PATCH 08/11] perf tool: precise mode requires exclude_guest David Ahern
2012-07-23 18:13   ` Arnaldo Carvalho de Melo
2012-07-24 14:20     ` David Ahern
2012-07-24 16:15       ` Robert Richter
2012-07-24 17:28         ` David Ahern
2012-07-24 18:03           ` Arnaldo Carvalho de Melo
2012-07-26  5:16             ` David Ahern
2012-07-26  8:08               ` Peter Zijlstra
2012-08-03 13:51                 ` Robert Richter [this message]
2012-08-15 15:22                   ` David Ahern
2012-09-05 15:43                   ` David Ahern
2012-09-05 22:07                     ` Peter Zijlstra
2012-09-06 17:44                       ` Robert Richter
2012-09-06 18:19                         ` David Ahern
2012-07-25 20:35         ` Peter Zijlstra
2012-07-26  5:50           ` Gleb Natapov
2012-07-26  8:07             ` Peter Zijlstra
2012-07-26  8:10               ` Gleb Natapov
2012-07-26  9:07               ` Robert Richter
2012-08-02 16:06             ` David Ahern
2012-09-06 19:02   ` Robert Richter
2012-09-06 19:17     ` David Ahern
2012-09-06 19:56       ` Robert Richter
2012-09-07 16:41         ` [PATCH] perf, ibs: Check syscall attribute flags Robert Richter
2012-09-07 16:50           ` David Ahern
2012-09-07 17:07             ` Robert Richter
2012-09-07 17:11               ` David Ahern
2012-09-07 17:20                 ` David Ahern
2012-09-10 13:07                   ` Robert Richter
2012-09-10 15:37                     ` Arnaldo Carvalho de Melo
2012-09-07 17:25                 ` Robert Richter
2012-09-07 16:56           ` Peter Zijlstra
2012-09-07 17:18             ` Robert Richter
2012-09-07 19:11               ` Peter Zijlstra
2012-09-10  9:30             ` [PATCH -v2] " Robert Richter
2012-09-14  6:17               ` [tip:perf/urgent] perf/x86/ibs: " tip-bot for Robert Richter
2012-07-20 23:25 ` [PATCH 09/11] perf top: error handling for counter creation should parallel perf-record David Ahern
2012-07-23 18:15   ` Arnaldo Carvalho de Melo
2012-07-24 14:01     ` David Ahern
2012-07-24 14:39       ` Arnaldo Carvalho de Melo
2012-07-20 23:25 ` [PATCH 10/11] perf tool: give user better message if precise is not supported David Ahern
2012-07-21 10:40   ` Namhyung Kim
2012-07-22  2:28     ` David Ahern
2012-07-20 23:25 ` [PATCH 11/11] perf kvm top: limit guest kernel info message to once David Ahern
2012-07-23 18:17   ` Arnaldo Carvalho de Melo
2012-07-24 14:02     ` David Ahern
2012-07-24 14:40       ` Arnaldo Carvalho de Melo

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=20120803135117.GC3732@erda.amd.com \
    --to=robert.richter@amd.com \
    --cc=acme@ghostprotocols.net \
    --cc=dsahern@gmail.com \
    --cc=fweisbec@gmail.com \
    --cc=gleb@redhat.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.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;
as well as URLs for NNTP newsgroup(s).