LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Liang, Kan" <kan.liang@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: acme@kernel.org, mingo@redhat.com, linux-kernel@vger.kernel.org,
	tglx@linutronix.de, jolsa@kernel.org, eranian@google.com,
	alexander.shishkin@linux.intel.com, ak@linux.intel.com
Subject: Re: [PATCH 4/9] perf/x86/intel: Support hardware TopDown metrics
Date: Wed, 29 May 2019 10:41:49 -0400	[thread overview]
Message-ID: <8ef9f52d-0ec6-8093-d1ea-ffa3450699fa@linux.intel.com> (raw)
In-Reply-To: <20190529073421.GZ2623@hirez.programming.kicks-ass.net>



On 5/29/2019 3:34 AM, Peter Zijlstra wrote:
>>>> +			wrmsrl(MSR_PERF_METRICS, 0);
>>>> +			wrmsrl(MSR_CORE_PERF_FIXED_CTR3, 0);
>>> I don't get this, overflow happens on when we flip sign, so why is
>>> programming 0 a sane thing to do?
>> Reset the counters (programming 0) don't trigger overflow.
> Right, so why then do you allow creating this thing as
> is_sampling_event() ?

Perf metrics event doesn't support sampling.
SLOTs/FIXED3 event + Perf metrics event don't support sampling either.

Only the case SLOTs/FIXED3 event without Perf metrics events support 
sampling. But the case doesn't reach this code path. It is handled by 
the normal routing, like other sampling events.

So there is no sampling event in this code path.

> 
>> We have to reset both registers for each read to avoid the known
>> PERF_METRICS issue.
> 'the known PERF_METRICS issue' is unknown to me and any other reader.
> 
>>>> +	metric = (cpuc->last_metric >> ((hwc->idx - INTEL_PMC_IDX_FIXED_METRIC_BASE)*8)) & 0xff;
>>>> +	last = (((metric * 0xffff) >> 8) * cpuc->last_slots) >> 16;
>>> How is that cpuc->last_* crap not broken for NMIs ?
>> There should be no NMI for slots or metric events at the moment, because the
>> MSR_PERF_METRICS and MSR_CORE_PERF_FIXED_CTR3 are reset in first read.
>> Other NMIs will not touch the codes here.
> What happens if someone does: read(perf_fd) and then has the NMI hit?

Right, it looks like there is a corner case we cannot handle. The NMI 
hit can happen right after rdpmcl, but before reset.

My current thought is to disable the METRICS and SLOTs counter for first 
read. I will think about it.

Thanks,
Kan


  reply	other threads:[~2019-05-29 14:41 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-21 21:40 [PATCH 0/9] TopDown metrics support for Icelake kan.liang
2019-05-21 21:40 ` [PATCH 1/9] perf/core: Support a REMOVE transaction kan.liang
2019-05-21 21:40 ` [PATCH 2/9] perf/x86/intel: Basic support for metrics counters kan.liang
2019-05-28 12:05   ` Peter Zijlstra
2019-05-28 18:20     ` Liang, Kan
2019-05-28 12:15   ` Peter Zijlstra
2019-05-28 18:21     ` Liang, Kan
2019-05-29  7:28       ` Peter Zijlstra
2019-05-29 14:40         ` Liang, Kan
2019-05-29 16:46           ` Peter Zijlstra
2019-05-29  8:14   ` Peter Zijlstra
2019-05-21 21:40 ` [PATCH 3/9] perf/x86/intel: Support overflows on SLOTS kan.liang
2019-05-28 12:20   ` Peter Zijlstra
2019-05-28 18:22     ` Liang, Kan
2019-05-21 21:40 ` [PATCH 4/9] perf/x86/intel: Support hardware TopDown metrics kan.liang
2019-05-28 12:43   ` Peter Zijlstra
2019-05-28 18:23     ` Liang, Kan
2019-05-29  7:30       ` Peter Zijlstra
2019-05-28 12:53   ` Peter Zijlstra
2019-05-28 12:56   ` Peter Zijlstra
2019-05-28 13:32     ` Peter Zijlstra
2019-05-28 13:30   ` Peter Zijlstra
2019-05-28 18:24     ` Liang, Kan
2019-05-29  7:34       ` Peter Zijlstra
2019-05-29 14:41         ` Liang, Kan [this message]
2019-05-28 13:43   ` Peter Zijlstra
2019-05-28 18:24     ` Liang, Kan
2019-05-29  7:54       ` Peter Zijlstra
2019-05-29 14:42         ` Liang, Kan
2019-05-29 16:58           ` Peter Zijlstra
2019-06-04 20:39             ` Liang, Kan
2019-05-28 13:48   ` Peter Zijlstra
2019-05-28 18:24     ` Liang, Kan
2019-05-29  7:57       ` Peter Zijlstra
2019-05-29 14:42         ` Liang, Kan
2019-05-21 21:40 ` [PATCH 5/9] perf/x86/intel: Set correct weight for TopDown metrics events kan.liang
2019-05-28 13:50   ` Peter Zijlstra
2019-05-21 21:40 ` [PATCH 6/9] perf/x86/intel: Export new TopDown metrics events for Icelake kan.liang
2019-05-21 21:40 ` [PATCH 7/9] perf/x86/intel: Disable sampling read slots and topdown kan.liang
2019-05-28 13:52   ` Peter Zijlstra
2019-05-28 18:25     ` Liang, Kan
2019-05-29  7:58       ` Peter Zijlstra
2019-05-21 21:40 ` [PATCH 8/9] perf, tools, stat: Support new per thread TopDown metrics kan.liang
2019-05-21 21:40 ` [PATCH 9/9] perf, tools: Add documentation for topdown metrics kan.liang

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=8ef9f52d-0ec6-8093-d1ea-ffa3450699fa@linux.intel.com \
    --to=kan.liang@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH 4/9] perf/x86/intel: Support hardware TopDown metrics' \
    /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

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).