From: kajoljain <kjain@linux.ibm.com>
To: Sandipan Das <sandipan.das@amd.com>, Jiri Olsa <jolsa@redhat.com>,
acme@kernel.org
Cc: ravi.bangoria@amd.com, ananth.narayan@amd.com,
kim.phillips@amd.com, rrichter@amd.com,
linux-perf-users@vger.kernel.org, namhyung@kernel.org,
alexander.shishkin@linux.intel.com, mark.rutland@arm.com
Subject: Re: [RFC PATCH] tools/perf/x86: Use alternative format for AMD raw events
Date: Tue, 16 Nov 2021 10:08:48 +0530 [thread overview]
Message-ID: <bdbd0a4b-6221-ed90-0882-47248112ed3d@linux.ibm.com> (raw)
In-Reply-To: <9dc5cb10-2faf-a17b-ad4b-a22dbe688209@amd.com>
On 11/15/21 3:28 PM, Sandipan Das wrote:
>
> On 11/14/2021 10:11 PM, Jiri Olsa wrote:
>> On Thu, Nov 11, 2021 at 06:26:46PM +0530, Sandipan Das wrote:
>>
>> SNIP
>>
>>> Before:
>>>
>>> $ sudo perf --debug perf-event-open stat -e ic_tag_hit_miss.all_instruction_cache_accesses sleep 1
>>> ------------------------------------------------------------
>>> perf_event_attr:
>>> type 4
>>> size 128
>>> config 0x100001f8e
>>> sample_type IDENTIFIER
>>> read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>>> disabled 1
>>> inherit 1
>>> enable_on_exec 1
>>> exclude_guest 1
>>> ------------------------------------------------------------
>>> [...]
>>>
>>> $ sudo perf --debug perf-event-open stat -e r1f18e sleep 1
>>> ------------------------------------------------------------
>>> perf_event_attr:
>>> type 4
>>> size 128
>>> config 0x1f18e
>>> sample_type IDENTIFIER
>>> read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>>> disabled 1
>>> inherit 1
>>> enable_on_exec 1
>>> exclude_guest 1
>>> ------------------------------------------------------------
>>> [...]
>>>
>>> $ sudo perf --debug perf-event-open stat -e r100001f8e sleep 1
>>> ------------------------------------------------------------
>>> perf_event_attr:
>>> type 4
>>> size 128
>>> config 0x100001f8e
>>> sample_type IDENTIFIER
>>> read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>>> disabled 1
>>> inherit 1
>>> enable_on_exec 1
>>> exclude_guest 1
>>> ------------------------------------------------------------
>>> [...]
>>>
>>> After:
>>>
>>> $ sudo perf --debug perf-event-open stat -e ic_tag_hit_miss.all_instruction_cache_accesses sleep 1
>>> ------------------------------------------------------------
>>> perf_event_attr:
>>> type 4
>>> size 128
>>> config 0x100001f8e
>>> sample_type IDENTIFIER
>>> read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>>> disabled 1
>>> inherit 1
>>> enable_on_exec 1
>>> exclude_guest 1
>>> ------------------------------------------------------------
>>> [...]
>>>
>>> $ sudo perf --debug perf-event-open stat -e r1f18e sleep 1
>>> ------------------------------------------------------------
>>> perf_event_attr:
>>> type 4
>>> size 128
>>> config 0x100001f8e
>>> sample_type IDENTIFIER
>>> read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>>> disabled 1
>>> inherit 1
>>> enable_on_exec 1
>>> exclude_guest 1
>>> ------------------------------------------------------------
>>> [...]
>>>
>>> $ sudo perf --debug perf-event-open stat -e r100001f8e sleep 1
>>> ------------------------------------------------------------
>>> perf_event_attr:
>>> type 4
>>> size 128
>>> config 0x100001f8e
>>> sample_type IDENTIFIER
>>> read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>>> disabled 1
>>> inherit 1
>>> enable_on_exec 1
>>> exclude_guest 1
>>> ------------------------------------------------------------
>>> [...]
>>>
>>> It is understandable that raw events must follow the native
>>> PERF_CTL register format but it is worthwhile to pursue such
>>> alternative formats for usability?
>>
>> good question.. I see raw events as a way to put to config whatever
>> the user wants and IMO adding changes/quirks that silently changes
>> config could create confusion and angry users ;-)
>>
I agree, making quirks in config will only going to confuse users as
It's against the encodings we had in
/sys/bus/event_source/devices/<pmu>/format/*
>> I'd think that if user is composing/using raw events then using
>> directly r100001f8e is not such a big deal?
>>
>
> Sure. Some of the confusion may be due to the fact that the man pages
> for perf-{stat,record,top} state that raw events are "eventsel+umask".
> While that is technically true, it does not describe the encoding
> scheme (/sys/bus/event_source/devices/<pmu>/format/*)
>
> Would another option be to update the man pages with a reference to
> these sysfs files when describing raw events?
>
> Something like:
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 2d7df8703cf2..5dfdfeba594b 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -30,8 +30,10 @@ OPTIONS
>
> - a symbolic event name (use 'perf list' to list all events)
>
> - - a raw PMU event (eventsel+umask) in the form of rNNN where NNN is a
> - hexadecimal event descriptor.
> + - a raw PMU event (eventsel+umask) in the form of rN..NN where N..NN
> + is a hexadecimal value representing the raw encoding with the layout
> + of the corresponding event control register as defined by entries in
> + /sys/bus/event_sources/devices/<pmu>/format/*
Do we need to specify (eventsel+umask) in the raw event description? As
the format/fields totally depend on PMU and umask name convention is
specific to one arch.
Can we just update it to:
- a raw PMU event in the form of rNNN where NNN is a hexadecimal value
representing the raw encoding, with the layout of the corresponding
event control register as defined by entries in
/sys/bus/event_sources/devices/<pmu>/format/*
Thanks,
Kajol Jain
>
> - a symbolic or raw PMU event followed by an optional colon
> and a list of event modifiers, e.g., cpu-cycles:p. See the
>
>
>> perhaps there was some some discussion about this that I missed?
>>
>
> I am not aware of one but may be Arnaldo has come across something
> similar before?
>
>
> - Sandipan
>
next prev parent reply other threads:[~2021-11-16 4:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-11 12:56 [RFC PATCH] tools/perf/x86: Use alternative format for AMD raw events Sandipan Das
2021-11-14 16:41 ` Jiri Olsa
2021-11-15 9:58 ` Sandipan Das
2021-11-16 4:38 ` kajoljain [this message]
2021-11-16 16:01 ` Kim Phillips
2021-11-17 14:13 ` kajoljain
2021-11-18 7:28 ` Sandipan Das
2021-11-18 16:51 ` Kim Phillips
2021-11-28 15:36 ` Jiri Olsa
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=bdbd0a4b-6221-ed90-0882-47248112ed3d@linux.ibm.com \
--to=kjain@linux.ibm.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=ananth.narayan@amd.com \
--cc=jolsa@redhat.com \
--cc=kim.phillips@amd.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=namhyung@kernel.org \
--cc=ravi.bangoria@amd.com \
--cc=rrichter@amd.com \
--cc=sandipan.das@amd.com \
/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.