Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1351515 > unrolled thread

Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status on HSW

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Peter Zijlstra <peterz@infradead.org> - 2016-03-07 11:30 +0100
    Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Peter Zijlstra <peterz@infradead.org> - 2016-03-07 13:20 +0100
      Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Jiri Olsa <jolsa@redhat.com> - 2016-03-07 19:30 +0100
        Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Peter Zijlstra <peterz@infradead.org> - 2016-03-07 21:30 +0100
          Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Stephane Eranian <eranian@google.com> - 2016-03-08 22:10 +0100
            Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Peter Zijlstra <peterz@infradead.org> - 2016-03-08 22:10 +0100
              Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Stephane Eranian <eranian@google.com> - 2016-03-08 22:20 +0100
                Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Stephane Eranian <eranian@google.com> - 2016-03-09 06:40 +0100
                  Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Stephane Eranian <eranian@google.com> - 2016-03-09 06:50 +0100
                    Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Stephane Eranian <eranian@google.com> - 2016-03-09 18:50 +0100
                      Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Peter Zijlstra <peterz@infradead.org> - 2016-03-10 11:50 +0100
      Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Peter Zijlstra <peterz@infradead.org> - 2016-03-10 15:00 +0100
        Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status  on HSW Stephane Eranian <eranian@google.com> - 2016-03-10 17:20 +0100

#1351515 — Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status on HSW

On Thu, Mar 03, 2016 at 03:40:49PM -0800, Stephane Eranian wrote:
> On Thu, Mar 3, 2016 at 1:43 PM, Andi Kleen <ak@linux.intel.com> wrote:
> >
> > > +             /*
> > > +              * There are cases where, even though, the PEBS ovfl bit is set in
> > > +              * GLOBAL_OVF_STATUS, the PEBS events may also have their overflow bits
> > > +              * set for their counters. We must clear them here because they have
> > > +              * been processed as exact samples in the drain_pebs() routine. They
> > > +              * must not be processed again in the for_each_bit_set() loop for
> > > +              * regular samples below.
> > > +              */
> > > +             status &= ~cpuc->pebs_enabled;
> > > +             status &= x86_pmu.intel_ctrl | GLOBAL_STATUS_TRACE_TOPAPMI;
> >
> > If you just clear the bits here they will not be acked and stay around
> > forever in GLOBAL_STATUS, which causes other problems.
> >
> > You need a separate ack_status variable that contains all bits and is always
> > acked.
> 
> I understand that. You mean I need to that has all the bits that were set to
> call intel_pmu_ack_status().
> 
> But if you look at the code, and where I made the change, there is no more
> intel_pmu_ack_status() BEFORE you read the status again via
> intel_pmu_get_status().
> 
> So why would I need to keep another variable around?

I suspect Andi is having something along:

 lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org

applied to his tree.

[toc] | [next] | [standalone]

#1351571

On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:

> I suspect Andi is having something along:
> 
>  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
> 
> applied to his tree.

OK, I munged a bunch of patches together, please have a hard look at the
end result found in:

  git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core

[toc] | [prev] | [next] | [standalone]

#1351859

On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
> 
> > I suspect Andi is having something along:
> > 
> >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
> > 
> > applied to his tree.
> 
> OK, I munged a bunch of patches together, please have a hard look at the
> end result found in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
> 
> 

I needed declaration below to compile

jirka


---
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 0e37370..097e4c1 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -862,6 +862,8 @@ extern struct event_constraint intel_hsw_pebs_event_constraints[];
 
 extern struct event_constraint intel_skl_pebs_event_constraints[];
 
+extern struct event_constraint intel_bdw_pebs_event_constraints[];
+
 struct event_constraint *intel_pebs_constraints(struct perf_event *event);
 
 void intel_pmu_pebs_enable(struct perf_event *event);

[toc] | [prev] | [next] | [standalone]

#1351958

On Mon, Mar 07, 2016 at 07:27:31PM +0100, Jiri Olsa wrote:
> On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
> > 
> > > I suspect Andi is having something along:
> > > 
> > >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
> > > 
> > > applied to his tree.
> > 
> > OK, I munged a bunch of patches together, please have a hard look at the
> > end result found in:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
> > 
> > 
> 
> I needed declaration below to compile

Yes indeed, (because Monday) I only compiled part of the patch series
and then the build bot initial emails were all builds success.

I have since fixed this (and gotten a fail email from the build bot).

In any case, lots of fail, but it should be fixed if you've pulled from
that git branch in the last 5 hours or so.

[toc] | [prev] | [next] | [standalone]

#1353404

hi,

On Mon, Mar 7, 2016 at 12:25 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Mar 07, 2016 at 07:27:31PM +0100, Jiri Olsa wrote:
> > On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
> > > On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
> > >
> > > > I suspect Andi is having something along:
> > > >
> > > >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
> > > >
> > > > applied to his tree.
> > >
> > > OK, I munged a bunch of patches together, please have a hard look at the
> > > end result found in:
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
> > >

I ran this kernel on Haswell. Even with Andi's fixes the problem I identified is
still there, so my patch is still needed.

>
> > >
> >
> > I needed declaration below to compile
>
> Yes indeed, (because Monday) I only compiled part of the patch series
> and then the build bot initial emails were all builds success.
>
> I have since fixed this (and gotten a fail email from the build bot).
>
> In any case, lots of fail, but it should be fixed if you've pulled from
> that git branch in the last 5 hours or so.

[toc] | [prev] | [next] | [standalone]

#1353406

On Tue, Mar 08, 2016 at 12:59:23PM -0800, Stephane Eranian wrote:
> hi,
> 
> On Mon, Mar 7, 2016 at 12:25 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Mon, Mar 07, 2016 at 07:27:31PM +0100, Jiri Olsa wrote:
> > > On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
> > > > On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
> > > >
> > > > > I suspect Andi is having something along:
> > > > >
> > > > >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
> > > > >
> > > > > applied to his tree.
> > > >
> > > > OK, I munged a bunch of patches together, please have a hard look at the
> > > > end result found in:
> > > >
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
> > > >
> 
> I ran this kernel on Haswell. Even with Andi's fixes the problem I identified is
> still there, so my patch is still needed.

Right, your patch should be included in that kernel, or did I make a
royal mess of things?

I put Andi's late status ack on top of your patch.

Also note, Ingo merged most of those patches today, all except the top
3, because Andi wanted to double check something.

[toc] | [prev] | [next] | [standalone]

#1353417

Hi,

On Tue, Mar 8, 2016 at 1:07 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Mar 08, 2016 at 12:59:23PM -0800, Stephane Eranian wrote:
>> hi,
>>
>> On Mon, Mar 7, 2016 at 12:25 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> >
>> > On Mon, Mar 07, 2016 at 07:27:31PM +0100, Jiri Olsa wrote:
>> > > On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
>> > > > On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
>> > > >
>> > > > > I suspect Andi is having something along:
>> > > > >
>> > > > >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
>> > > > >
>> > > > > applied to his tree.
>> > > >
>> > > > OK, I munged a bunch of patches together, please have a hard look at the
>> > > > end result found in:
>> > > >
>> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
>> > > >
>>
>> I ran this kernel on Haswell. Even with Andi's fixes the problem I identified is
>> still there, so my patch is still needed.
>
> Right, your patch should be included in that kernel, or did I make a
> royal mess of things?
>
No, it is as expected for the OVF PMI fix.

> I put Andi's late status ack on top of your patch.
>
> Also note, Ingo merged most of those patches today, all except the top
> 3, because Andi wanted to double check something.

Ok. I will try on SKL today.

[toc] | [prev] | [next] | [standalone]

#1353828

On Tue, Mar 8, 2016 at 1:13 PM, Stephane Eranian <eranian@google.com> wrote:
> Hi,
>
> On Tue, Mar 8, 2016 at 1:07 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Tue, Mar 08, 2016 at 12:59:23PM -0800, Stephane Eranian wrote:
>>> hi,
>>>
>>> On Mon, Mar 7, 2016 at 12:25 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>> >
>>> > On Mon, Mar 07, 2016 at 07:27:31PM +0100, Jiri Olsa wrote:
>>> > > On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
>>> > > > On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
>>> > > >
>>> > > > > I suspect Andi is having something along:
>>> > > > >
>>> > > > >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
>>> > > > >
>>> > > > > applied to his tree.
>>> > > >
>>> > > > OK, I munged a bunch of patches together, please have a hard look at the
>>> > > > end result found in:
>>> > > >
>>> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
>>> > > >
>>>
>>> I ran this kernel on Haswell. Even with Andi's fixes the problem I identified is
>>> still there, so my patch is still needed.
>>
>> Right, your patch should be included in that kernel, or did I make a
>> royal mess of things?
>>
> No, it is as expected for the OVF PMI fix.
>
>> I put Andi's late status ack on top of your patch.
>>
Ok, I ran into a problem on Broadwell with your branch with Andi's
patches. I see
a problem which had disappeared since SandyBridge:

11551.128422] ------------[ cut here ]------------
[11551.128435] WARNING: CPU: 3 PID: 12114 at
arch/x86/events/intel/core.c:1868 intel_pmu_handle_irq+0x2da/0x4b0()
[11551.128437] perfevents: irq loop stuck!
[11551.128469]  <NMI>  [<ffffffff81663975>] dump_stack+0x4d/0x63
[11551.128479]  [<ffffffff810b8657>] warn_slowpath_common+0x97/0xe0
[11551.128482]  [<ffffffff810b8756>] warn_slowpath_fmt+0x46/0x50
[11551.128486]  [<ffffffff8100b6ca>] intel_pmu_handle_irq+0x2da/0x4b0
[11551.128491]  [<ffffffff81004569>] perf_event_nmi_handler+0x39/0x60
[11551.128494]  [<ffffffff8107be61>] nmi_handle+0x61/0x110
[11551.128497]  [<ffffffff8107c684>] default_do_nmi+0x44/0x110
[11551.128500]  [<ffffffff8107c827>] do_nmi+0xd7/0x140
[11551.128504]  [<ffffffff8166e127>] end_repeat_nmi+0x1a/0x1e
[11551.128507]  [<ffffffff81009dd6>] ? native_write_msr+0x6/0x30
[11551.128510]  [<ffffffff81009dd6>] ? native_write_msr+0x6/0x30
[11551.128514]  [<ffffffff81009dd6>] ? native_write_msr+0x6/0x30
[11551.128515]  <<EOE>>  [<ffffffff8100b385>] ?
intel_pmu_enable_event+0x215/0x230
[11551.128520]  [<ffffffff81005a0d>] x86_pmu_start+0x8d/0x120
[11551.128523]  [<ffffffff810061db>] x86_pmu_enable+0x27b/0x2f0
[11551.128527]  [<ffffffff8118d63d>] perf_pmu_enable+0x1d/0x30
[11551.128530]  [<ffffffff81191bca>] ctx_resched+0x5a/0x70
[11551.128532]  [<ffffffff81191d8c>] __perf_event_enable+0x1ac/0x210
[11551.128537]  [<ffffffff81188f81>] event_function+0xa1/0x170
[11551.128540]  [<ffffffff811899b0>] ? perf_duration_warn+0x70/0x70
[11551.128543]  [<ffffffff811899f7>] remote_function+0x47/0x60
[11551.128547]  [<ffffffff8112a178>] generic_exec_single+0xa8/0xb0
[11551.128550]  [<ffffffff811899b0>] ? perf_duration_warn+0x70/0x70
[11551.128553]  [<ffffffff811899b0>] ? perf_duration_warn+0x70/0x70
[11551.128555]  [<ffffffff8112a298>] smp_call_function_single+0xa8/0x100
[11551.128559]  [<ffffffff8118aec4>] event_function_call+0x84/0x100
[11551.128561]  [<ffffffff81191be0>] ? ctx_resched+0x70/0x70
[11551.128564]  [<ffffffff81191be0>] ? ctx_resched+0x70/0x70
[11551.128566]  [<ffffffff81188ee0>] ? perf_ctx_lock+0x30/0x30
[11551.128570]  [<ffffffff8118b050>] _perf_event_enable+0x60/0x80
[11551.128572]  [<ffffffff8118fc61>] perf_ioctl+0x271/0x3e0

The infinite loop in the irq handler!

But here it seems there is a race with a perf_events ioctl() to likely
reset the period.
I am not using the perf tool here just running a self-monitoring task.


>> Also note, Ingo merged most of those patches today, all except the top
>> 3, because Andi wanted to double check something.
>

[toc] | [prev] | [next] | [standalone]

#1353834

On Tue, Mar 8, 2016 at 9:34 PM, Stephane Eranian <eranian@google.com> wrote:
> On Tue, Mar 8, 2016 at 1:13 PM, Stephane Eranian <eranian@google.com> wrote:
>> Hi,
>>
>> On Tue, Mar 8, 2016 at 1:07 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>> On Tue, Mar 08, 2016 at 12:59:23PM -0800, Stephane Eranian wrote:
>>>> hi,
>>>>
>>>> On Mon, Mar 7, 2016 at 12:25 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>>> >
>>>> > On Mon, Mar 07, 2016 at 07:27:31PM +0100, Jiri Olsa wrote:
>>>> > > On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
>>>> > > > On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
>>>> > > >
>>>> > > > > I suspect Andi is having something along:
>>>> > > > >
>>>> > > > >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
>>>> > > > >
>>>> > > > > applied to his tree.
>>>> > > >
>>>> > > > OK, I munged a bunch of patches together, please have a hard look at the
>>>> > > > end result found in:
>>>> > > >
>>>> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
>>>> > > >
>>>>
>>>> I ran this kernel on Haswell. Even with Andi's fixes the problem I identified is
>>>> still there, so my patch is still needed.
>>>
>>> Right, your patch should be included in that kernel, or did I make a
>>> royal mess of things?
>>>
>> No, it is as expected for the OVF PMI fix.
>>
>>> I put Andi's late status ack on top of your patch.
>>>
> Ok, I ran into a problem on Broadwell with your branch with Andi's
> patches. I see

Sorry this is with tip.git and not your branch. Will try with it too.

> a problem which had disappeared since SandyBridge:
>
> 11551.128422] ------------[ cut here ]------------
> [11551.128435] WARNING: CPU: 3 PID: 12114 at
> arch/x86/events/intel/core.c:1868 intel_pmu_handle_irq+0x2da/0x4b0()
> [11551.128437] perfevents: irq loop stuck!
> [11551.128469]  <NMI>  [<ffffffff81663975>] dump_stack+0x4d/0x63
> [11551.128479]  [<ffffffff810b8657>] warn_slowpath_common+0x97/0xe0
> [11551.128482]  [<ffffffff810b8756>] warn_slowpath_fmt+0x46/0x50
> [11551.128486]  [<ffffffff8100b6ca>] intel_pmu_handle_irq+0x2da/0x4b0
> [11551.128491]  [<ffffffff81004569>] perf_event_nmi_handler+0x39/0x60
> [11551.128494]  [<ffffffff8107be61>] nmi_handle+0x61/0x110
> [11551.128497]  [<ffffffff8107c684>] default_do_nmi+0x44/0x110
> [11551.128500]  [<ffffffff8107c827>] do_nmi+0xd7/0x140
> [11551.128504]  [<ffffffff8166e127>] end_repeat_nmi+0x1a/0x1e
> [11551.128507]  [<ffffffff81009dd6>] ? native_write_msr+0x6/0x30
> [11551.128510]  [<ffffffff81009dd6>] ? native_write_msr+0x6/0x30
> [11551.128514]  [<ffffffff81009dd6>] ? native_write_msr+0x6/0x30
> [11551.128515]  <<EOE>>  [<ffffffff8100b385>] ?
> intel_pmu_enable_event+0x215/0x230
> [11551.128520]  [<ffffffff81005a0d>] x86_pmu_start+0x8d/0x120
> [11551.128523]  [<ffffffff810061db>] x86_pmu_enable+0x27b/0x2f0
> [11551.128527]  [<ffffffff8118d63d>] perf_pmu_enable+0x1d/0x30
> [11551.128530]  [<ffffffff81191bca>] ctx_resched+0x5a/0x70
> [11551.128532]  [<ffffffff81191d8c>] __perf_event_enable+0x1ac/0x210
> [11551.128537]  [<ffffffff81188f81>] event_function+0xa1/0x170
> [11551.128540]  [<ffffffff811899b0>] ? perf_duration_warn+0x70/0x70
> [11551.128543]  [<ffffffff811899f7>] remote_function+0x47/0x60
> [11551.128547]  [<ffffffff8112a178>] generic_exec_single+0xa8/0xb0
> [11551.128550]  [<ffffffff811899b0>] ? perf_duration_warn+0x70/0x70
> [11551.128553]  [<ffffffff811899b0>] ? perf_duration_warn+0x70/0x70
> [11551.128555]  [<ffffffff8112a298>] smp_call_function_single+0xa8/0x100
> [11551.128559]  [<ffffffff8118aec4>] event_function_call+0x84/0x100
> [11551.128561]  [<ffffffff81191be0>] ? ctx_resched+0x70/0x70
> [11551.128564]  [<ffffffff81191be0>] ? ctx_resched+0x70/0x70
> [11551.128566]  [<ffffffff81188ee0>] ? perf_ctx_lock+0x30/0x30
> [11551.128570]  [<ffffffff8118b050>] _perf_event_enable+0x60/0x80
> [11551.128572]  [<ffffffff8118fc61>] perf_ioctl+0x271/0x3e0
>
> The infinite loop in the irq handler!
>
> But here it seems there is a race with a perf_events ioctl() to likely
> reset the period.
> I am not using the perf tool here just running a self-monitoring task.
>
>
>>> Also note, Ingo merged most of those patches today, all except the top
>>> 3, because Andi wanted to double check something.
>>

[toc] | [prev] | [next] | [standalone]

#1354312

On Tue, Mar 8, 2016 at 9:44 PM, Stephane Eranian <eranian@google.com> wrote:
> On Tue, Mar 8, 2016 at 9:34 PM, Stephane Eranian <eranian@google.com> wrote:
>> On Tue, Mar 8, 2016 at 1:13 PM, Stephane Eranian <eranian@google.com> wrote:
>>> Hi,
>>>
>>> On Tue, Mar 8, 2016 at 1:07 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>>> On Tue, Mar 08, 2016 at 12:59:23PM -0800, Stephane Eranian wrote:
>>>>> hi,
>>>>>
>>>>> On Mon, Mar 7, 2016 at 12:25 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>>>> >
>>>>> > On Mon, Mar 07, 2016 at 07:27:31PM +0100, Jiri Olsa wrote:
>>>>> > > On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
>>>>> > > > On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
>>>>> > > >
>>>>> > > > > I suspect Andi is having something along:
>>>>> > > > >
>>>>> > > > >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
>>>>> > > > >
>>>>> > > > > applied to his tree.
>>>>> > > >
>>>>> > > > OK, I munged a bunch of patches together, please have a hard look at the
>>>>> > > > end result found in:
>>>>> > > >
>>>>> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
>>>>> > > >
>>>>>
>>>>> I ran this kernel on Haswell. Even with Andi's fixes the problem I identified is
>>>>> still there, so my patch is still needed.
>>>>
>>>> Right, your patch should be included in that kernel, or did I make a
>>>> royal mess of things?
>>>>
>>> No, it is as expected for the OVF PMI fix.
>>>
>>>> I put Andi's late status ack on top of your patch.
>>>>
>> Ok, I ran into a problem on Broadwell with your branch with Andi's
>> patches. I see
>
> Sorry this is with tip.git and not your branch. Will try with it too.

With your queue.tip perf/core branch, I run into another problem.
I am monitoring with 2 PEBS events and I have the NMI watchdog enabled.

I see non-EXACT PEBS records again, despite my change (which is in).
I tracked it down to the following issue after the testing of bit 62:

[31137.273061] CPU71 status=0x200000001 orig_status=0x200000001 bit62=0

The IRQ handler is called because the fixed counter for the NMI has overflowed
and it sees this in bit 33, but it also sees that one of the PEBS
events has also
overflowed, yet bit 62 is not set. Therefore both overflows are
treated as regular
and the drain_pebs() is not called generating a non-EXACT record for the PEBS
counter (counter 0). So something is wrong still and this is on Broadwell.

First, I don't understand why the OVF bit for counter 0 is set. It
should not according
to specs because the counter is in PEBS mode. There must be a race there. So we
have to handle it by relying on cpuc->pebs_enabled. I will try that.
We likely also
need to force OVF bit 62 to 1 so we can ack it in the end (and in case
it gets set).

[toc] | [prev] | [next] | [standalone]

#1354997

On Wed, Mar 09, 2016 at 09:40:07AM -0800, Stephane Eranian wrote:
> With your queue.tip perf/core branch, I run into another problem.
> I am monitoring with 2 PEBS events and I have the NMI watchdog enabled.
> 
> I see non-EXACT PEBS records again, despite my change (which is in).
> I tracked it down to the following issue after the testing of bit 62:
> 
> [31137.273061] CPU71 status=0x200000001 orig_status=0x200000001 bit62=0
> 
> The IRQ handler is called because the fixed counter for the NMI has overflowed
> and it sees this in bit 33, but it also sees that one of the PEBS
> events has also
> overflowed, yet bit 62 is not set. Therefore both overflows are
> treated as regular
> and the drain_pebs() is not called generating a non-EXACT record for the PEBS
> counter (counter 0). So something is wrong still and this is on Broadwell.
> 
> First, I don't understand why the OVF bit for counter 0 is set. It
> should not according
> to specs because the counter is in PEBS mode. There must be a race there. So we
> have to handle it by relying on cpuc->pebs_enabled. I will try that.
> We likely also
> need to force OVF bit 62 to 1 so we can ack it in the end (and in case
> it gets set).

How about we make the clear of pebs_enabled unconditional?

---
 arch/x86/events/intel/core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 68fa55b4d42e..dc9579665425 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -1883,6 +1883,16 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 	status &= ~(GLOBAL_STATUS_COND_CHG |
 		    GLOBAL_STATUS_ASIF |
 		    GLOBAL_STATUS_LBRS_FROZEN);
+	/*
+	 * There are cases where, even though, the PEBS ovfl bit is set
+	 * in GLOBAL_OVF_STATUS, the PEBS events may also have their
+	 * overflow bits set for their counters. We must clear them
+	 * here because they have been processed as exact samples in
+	 * the drain_pebs() routine. They must not be processed again
+	 * in the for_each_bit_set() loop for regular samples below.
+	 */
+	status &= ~cpuc->pebs_enabled;
+
 	if (!status)
 		goto done;
 
@@ -1892,16 +1902,6 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 	if (__test_and_clear_bit(62, (unsigned long *)&status)) {
 		handled++;
 		x86_pmu.drain_pebs(regs);
-		/*
-		 * There are cases where, even though, the PEBS ovfl bit is set
-		 * in GLOBAL_OVF_STATUS, the PEBS events may also have their
-		 * overflow bits set for their counters. We must clear them
-		 * here because they have been processed as exact samples in
-		 * the drain_pebs() routine. They must not be processed again
-		 * in the for_each_bit_set() loop for regular samples below.
-		 */
-		status &= ~cpuc->pebs_enabled;
-		status &= x86_pmu.intel_ctrl | GLOBAL_STATUS_TRACE_TOPAPMI;
 	}
 
 	/*

[toc] | [prev] | [next] | [standalone]

#1355130

On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
> 
> > I suspect Andi is having something along:
> > 
> >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
> > 
> > applied to his tree.
> 
> OK, I munged a bunch of patches together, please have a hard look at the
> end result found in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core

So while running tip/master I just triggered the "irq loop stuck" warn
on my IVB, with status==1, and PMC0 didn't look weird at all.

I've not seen that for a fair while. But it does show it still happens
on not quite antique hardware.

[toc] | [prev] | [next] | [standalone]

#1355247

On Thu, Mar 10, 2016 at 5:53 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 07, 2016 at 01:18:40PM +0100, Peter Zijlstra wrote:
>> On Mon, Mar 07, 2016 at 11:24:13AM +0100, Peter Zijlstra wrote:
>>
>> > I suspect Andi is having something along:
>> >
>> >  lkml.kernel.org/r/1445458568-16956-1-git-send-email-andi@firstfloor.org
>> >
>> > applied to his tree.
>>
>> OK, I munged a bunch of patches together, please have a hard look at the
>> end result found in:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
>
> So while running tip/master I just triggered the "irq loop stuck" warn
> on my IVB, with status==1, and PMC0 didn't look weird at all.
>
> I've not seen that for a fair while. But it does show it still happens
> on not quite antique hardware.

Yes, as I said, tip.git is missing something, with Andi's missing 3
patches I do not get
that. But with them, I was getting the problem I described earlier, so
I just posted a
patch to handle that particular case where bit 62 is not set, yet PEBS
counters show
overflow. Test it.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web