All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Stephane Eranian <eranian@google.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>, x86 <x86@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Kan Liang <kan.liang@linux.intel.com>,
	John Sperbeck <jsperbeck@google.com>,
	"Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
	Andi Kleen <ak@linux.intel.com>
Subject: Re: [RFC] perf/x86: Fix a warning on x86_pmu_stop()
Date: Wed, 25 Nov 2020 08:36:46 +0100	[thread overview]
Message-ID: <20201125073646.GL2414@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <CABPqkBSvcj1c5-SO4PQLc2KLLf7_0bW4ftUyvFobyd0OVAmLrA@mail.gmail.com>

On Tue, Nov 24, 2020 at 12:19:34AM -0800, Stephane Eranian wrote:
> Hi,
> 
> Another remark on the PEBS drainage code, it seems to me like a test
> is not quite correct:
> intel_pmu_drain_pebs_nhm()
> {
> ...
>                if (p->status != (1ULL << bit)) {
>                         for_each_set_bit(i, (unsigned long *)&pebs_status, size)
>                                 error[i]++;
>                         continue;
>                 }
> 
> The kernel cannot disambiguate when 2+ PEBS counters overflow at the
> same time. This is what the comment for this code suggests.
> However, I see the comparison is done with the unfiltered p->status
> which is a copy of  IA32_PERF_GLOBAL_STATUS at the time of
> the sample. This register contains more than the PEBS counter overflow
> bits. It also includes many other bits which could also be set.
> 
> Shouldn't this test use pebs_status instead (which covers only the
> PEBS counters)?

Hmm, yes, think so.

  reply	other threads:[~2020-11-25  7:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21  2:50 [RFC] perf/x86: Fix a warning on x86_pmu_stop() Namhyung Kim
2020-11-23 14:23 ` Peter Zijlstra
2020-11-24  5:01   ` Namhyung Kim
2020-11-24  8:09     ` Peter Zijlstra
2020-11-24  8:19       ` Stephane Eranian
2020-11-25  7:36         ` Peter Zijlstra [this message]
2020-11-25  7:22       ` Namhyung Kim

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=20201125073646.GL2414@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=Thomas.Lendacky@amd.com \
    --cc=ak@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=eranian@google.com \
    --cc=hpa@zytor.com \
    --cc=jsperbeck@google.com \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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.