All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: ak@linux.intel.com, Michael Ellerman <michaele@au1.ibm.com>,
	peterz@infradead.org, linux-kernel@vger.kernel.org,
	eranian@google.com, dev@codyps.com,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org,
	Anshuman Khandual <khandual@linux.vnet.ibm.com>
Subject: Re: [PATCH v4 01/10] tools/perf: support parsing parameterized events
Date: Thu, 25 Sep 2014 19:25:20 -0700	[thread overview]
Message-ID: <20140926022520.GA8997@us.ibm.com> (raw)
In-Reply-To: <20140925085858.GA20915@krava.brq.redhat.com>

Jiri Olsa [jolsa@redhat.com] wrote:
| On Wed, Sep 24, 2014 at 12:27:15PM -0700, Sukadev Bhattiprolu wrote:
| > From: Cody P Schafer <cody@linux.vnet.ibm.com>
| > 
| > Enable event specification like:
| > 
| > 	pmu/event_name,param1=0x1,param2=0x4/
| > 
| > Assuming that
| > 
| > 	/sys/bus/event_source/devices/pmu/events/event_name
| > 
| > Contains something like
| > 
| > 	param2=foo,bar=1,param1=baz
| 
| hum, so what happened to the '?' ... AFAIU from out last discussion,
| you wanted to mark terms which are mandatory and user must provide
| values for them.. and I thought the decision was to have following
| alias record:
| 
|   $ cat /sys/bus/event_source/devices/pmu/events/event_name
|   param2=?,bar=1,param1=?
| 
| while perf would scream if any of param1/2 wasnt filled like for:
|   pmu/event_name,param1=0x1/

Sorry, I meant to make perf list consistent with sysfs.

Consider these two sysfs entries:

	$ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE 
	domain=0x2,offset=0xe0,starting_index=core,lpar=0x0

	$ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE 
	domain=0x3,offset=0xe0,starting_index=vcpu,lpar=sibling_guest_id

In the first one, starting_index refers to a 'core' while in the second
it refers to a vcpu. This serves as a "hint" for the parameter's meaning.

By replacing both with 'starting_index=?' we lose that hint.

Should we fix both sysfs and 'perf list' to say

	starting_index=?core 

Sukadev

WARNING: multiple messages have this Message-ID (diff)
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	ak@linux.intel.com, peterz@infradead.org, eranian@google.com,
	Paul Mackerras <paulus@samba.org>,
	dev@codyps.com, Michael Ellerman <michaele@au1.ibm.com>,
	Anshuman Khandual <khandual@linux.vnet.ibm.com>,
	hbabu@us.ibm.com, linux-kernel@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v4 01/10] tools/perf: support parsing parameterized events
Date: Thu, 25 Sep 2014 19:25:20 -0700	[thread overview]
Message-ID: <20140926022520.GA8997@us.ibm.com> (raw)
In-Reply-To: <20140925085858.GA20915@krava.brq.redhat.com>

Jiri Olsa [jolsa@redhat.com] wrote:
| On Wed, Sep 24, 2014 at 12:27:15PM -0700, Sukadev Bhattiprolu wrote:
| > From: Cody P Schafer <cody@linux.vnet.ibm.com>
| > 
| > Enable event specification like:
| > 
| > 	pmu/event_name,param1=0x1,param2=0x4/
| > 
| > Assuming that
| > 
| > 	/sys/bus/event_source/devices/pmu/events/event_name
| > 
| > Contains something like
| > 
| > 	param2=foo,bar=1,param1=baz
| 
| hum, so what happened to the '?' ... AFAIU from out last discussion,
| you wanted to mark terms which are mandatory and user must provide
| values for them.. and I thought the decision was to have following
| alias record:
| 
|   $ cat /sys/bus/event_source/devices/pmu/events/event_name
|   param2=?,bar=1,param1=?
| 
| while perf would scream if any of param1/2 wasnt filled like for:
|   pmu/event_name,param1=0x1/

Sorry, I meant to make perf list consistent with sysfs.

Consider these two sysfs entries:

	$ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE 
	domain=0x2,offset=0xe0,starting_index=core,lpar=0x0

	$ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE 
	domain=0x3,offset=0xe0,starting_index=vcpu,lpar=sibling_guest_id

In the first one, starting_index refers to a 'core' while in the second
it refers to a vcpu. This serves as a "hint" for the parameter's meaning.

By replacing both with 'starting_index=?' we lose that hint.

Should we fix both sysfs and 'perf list' to say

	starting_index=?core 

Sukadev


  reply	other threads:[~2014-09-26  2:25 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-24 19:27 [PATCH v4 00/10] Add support for parameterized events from sysfs Sukadev Bhattiprolu
2014-09-24 19:27 ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 01/10] tools/perf: support parsing parameterized events Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-25  8:58   ` Jiri Olsa
2014-09-25  8:58     ` Jiri Olsa
2014-09-26  2:25     ` Sukadev Bhattiprolu [this message]
2014-09-26  2:25       ` Sukadev Bhattiprolu
2014-10-01  9:06       ` Jiri Olsa
2014-10-01  9:06         ` Jiri Olsa
2014-10-01 15:59         ` Peter Zijlstra
2014-10-01 15:59           ` Peter Zijlstra
2014-09-24 19:27 ` [PATCH v4 02/10] tools/perf: extend format_alias() to include event parameters Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 03/10] perf: provide sysfs_show for struct perf_pmu_events_attr Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 04/10] powerpc/perf/hv-24x7: parse catalog and populate sysfs with events Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 05/10] perf: add PMU_EVENT_ATTR_STRING() helper Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 06/10] powerpc/perf/{hv-gpci, hv-common}: generate requests with counters annotated Sukadev Bhattiprolu
2014-09-24 19:27   ` [PATCH v4 06/10] powerpc/perf/{hv-gpci,hv-common}: " Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 07/10] powerpc/perf/hv-gpci: add the remaining gpci requests Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 08/10] perf Documentation: add event parameters Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 09/10] tools/perf: Document parameterized and symbolic events Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-24 19:27 ` [PATCH v4 10/10] powerpc/perf/hv-24x7: Document sysfs event description entries Sukadev Bhattiprolu
2014-09-24 19:27   ` Sukadev Bhattiprolu
2014-09-30 21:03   ` Cody P Schafer
2014-09-30 21:03     ` Cody P Schafer

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=20140926022520.GA8997@us.ibm.com \
    --to=sukadev@linux.vnet.ibm.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=dev@codyps.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=khandual@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=michaele@au1.ibm.com \
    --cc=paulus@samba.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 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.