lkml.org 
[lkml]   [2020]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2] perf evsel: Don't set sample_regs_intr/sample_regs_user for dummy event
    From
    Date
    Hi Jiri, Adrian,

    On 7/22/2020 7:08 PM, Jiri Olsa wrote:
    > On Wed, Jul 22, 2020 at 01:00:03PM +0800, Jin, Yao wrote:
    >
    > SNIP
    >
    >>>>
    >>>> If we use -IXMM0, the attr>sample_regs_intr will be set with
    >>>> PERF_REG_EXTENDED_MASK bit.
    >>>>
    >>>> It doesn't make sense to set attr->sample_regs_intr for a
    >>>> software dummy event.
    >>>>
    >>>> This patch adds dummy event checking before setting
    >>>> attr->sample_regs_intr and attr->sample_regs_user.
    >>>>
    >>>> After:
    >>>> # ./perf record -e cycles:p -IXMM0 -a -- sleep 1
    >>>> [ perf record: Woken up 1 times to write data ]
    >>>> [ perf record: Captured and wrote 0.413 MB perf.data (45 samples) ]
    >>>>
    >>>> v2:
    >>>> ---
    >>>> Rebase to perf/core
    >>>>
    >>>> Fixes: 0a892c1c9472 ("perf record: Add dummy event during system wide synthesis")
    >>>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
    >>>> ---
    >>>> tools/perf/util/evsel.c | 6 ++++--
    >>>> 1 file changed, 4 insertions(+), 2 deletions(-)
    >>>>
    >>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
    >>>> index 9aa51a65593d..11794d3b7879 100644
    >>>> --- a/tools/perf/util/evsel.c
    >>>> +++ b/tools/perf/util/evsel.c
    >>>> @@ -1014,12 +1014,14 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
    >>>> if (callchain && callchain->enabled && !evsel->no_aux_samples)
    >>>> evsel__config_callchain(evsel, opts, callchain);
    >>>> - if (opts->sample_intr_regs && !evsel->no_aux_samples) {
    >>>> + if (opts->sample_intr_regs && !evsel->no_aux_samples &&
    >>>> + !evsel__is_dummy_event(evsel)) {
    >>>
    >>> hum, I thought it'd look something like this:
    >>>
    >>> if (opts->sample_intr_regs && (!evsel->no_aux_samples || !evsel__is_dummy_event(evsel))
    >>>
    >>> but I'm not sure how no_aux_samples flag works exactly.. so it might be
    >>> correct.. just making sure ;-)
    >>>
    >>> cc-ing Adrian
    >>>
    >>> jirka
    >>>
    >>>
    >>
    >> no_aux_samples is set to false by default and it's only set to true by pt, right?
    >>
    >> So most of the time, !evsel->no_aux_samples is always true.
    >>
    >> if (opts->sample_intr_regs && (!evsel->no_aux_samples || !evsel__is_dummy_event(evsel)) {
    >> attr->sample_regs_intr = opts->sample_intr_regs;
    >> evsel__set_sample_bit(evsel, REGS_INTR);
    >> }
    >>
    >> So even if the evsel is dummy event, the condition check is true. :(
    >>
    >> Or maybe I misunderstand anything?
    >
    > I was just curious, because I did not follow the no_aux_samples
    > usage in detail.. so how about a case where:
    >
    > evsel->no_aux_samples == true and evsel__is_dummy_event(evsel) = false
    >
    > then the original condition will be false for non dummy event
    >
    > (opts->sample_intr_regs && !evsel->no_aux_samples && !evsel__is_dummy_event(evsel))
    >
    > is that ok?
    >

    I searched the perf source and found the no_aux_samples was only set to true in intel-pt.c. So I
    assume for the non-pt usage, the no_aux_samples is always false.

    For non-pt usage,
    (opts->sample_intr_regs && !evsel->no_aux_samples && !evsel__is_dummy_event(evsel)) is equal to
    (opts->sample_intr_regs && !evsel__is_dummy_event(evsel))

    For pt usage, we need to consider the case that evsel__is_dummy_event(evsel) is true or false.

    If evsel__is_dummy_event(evsel) is true:
    (opts->sample_intr_regs && !evsel->no_aux_samples && !evsel__is_dummy_event(evsel)) is false.
    It's expected.

    If evsel__is_dummy_event(evsel) is false:
    (opts->sample_intr_regs && !evsel->no_aux_samples && !evsel__is_dummy_event(evsel)) is equal to
    (opts->sample_intr_regs && !evsel->no_aux_samples)
    That's the current code logic.

    So I think the condition "(opts->sample_intr_regs && !evsel->no_aux_samples &&
    !evsel__is_dummy_event(evsel))" looks reasonable.

    Adrian, please correct me if I'm wrong here.

    Thanks
    Jin Yao

    > jirka
    >

    \
     
     \ /
      Last update: 2020-07-23 03:03    [W:3.894 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site