[PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Cyrill Gorcunov]

It's not enough to simply disable event on overflow the cpuc->active_mask
should be cleared as well otherwise counter may stall in "active" even
in real being already disabled (which potentially may lead to the situation
that user may not use this counter further).

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---

Ming, iirc you were testing this one, but without unknown nmi fix. So
patch from Don should fix unknown nmi issue, please give this patch a
shot again if you have some time (Don's patch is here
https://lkml.org/lkml/2011/4/18/404 but I think you have it already in mailbox).

 arch/x86/kernel/cpu/perf_event_p4.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event_p4.c
+++ linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
@@ -946,7 +946,7 @@ static int p4_pmu_handle_irq(struct pt_r
 		if (!x86_perf_event_set_period(event))
 			continue;
 		if (perf_event_overflow(event, 1, &data, regs))
-			p4_pmu_disable_event(event);
+			x86_pmu_stop(event, 0);
 	}

 	if (handled) {

-- 
    Cyrill

Re: [PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Lin Ming]

On Wed, 2011-04-20 at 23:13 +0800, Cyrill Gorcunov wrote:
> It's not enough to simply disable event on overflow the cpuc->active_mask
> should be cleared as well otherwise counter may stall in "active" even
> in real being already disabled (which potentially may lead to the situation
> that user may not use this counter further).
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
> 
> Ming, iirc you were testing this one, but without unknown nmi fix. So
> patch from Don should fix unknown nmi issue, please give this patch a
> shot again if you have some time (Don's patch is here
> https://lkml.org/lkml/2011/4/18/404 but I think you have it already in mailbox).

Tested the 2 patches with nmi watchdog enabled and no unknown nmi was
seen when running perf tool.

Nice.

Thanks,
Lin Ming

> 
>  arch/x86/kernel/cpu/perf_event_p4.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
> =====================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event_p4.c
> +++ linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
> @@ -946,7 +946,7 @@ static int p4_pmu_handle_irq(struct pt_r
>  		if (!x86_perf_event_set_period(event))
>  			continue;
>  		if (perf_event_overflow(event, 1, &data, regs))
> -			p4_pmu_disable_event(event);
> +			x86_pmu_stop(event, 0);
>  	}
> 
>  	if (handled) {
> 

Re: [PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Don Zickus]

On Wed, Apr 20, 2011 at 07:13:37PM +0400, Cyrill Gorcunov wrote:
> It's not enough to simply disable event on overflow the cpuc->active_mask
> should be cleared as well otherwise counter may stall in "active" even
> in real being already disabled (which potentially may lead to the situation
> that user may not use this counter further).

I also noticed this patch fixed some unknown NMIs on a P4 when I stressed
the box

Acked-by: Don Zickus <dzickus@redhat.com>

Re: [PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Ingo Molnar]

* Don Zickus <dzickus@redhat.com> wrote:

> On Wed, Apr 20, 2011 at 07:13:37PM +0400, Cyrill Gorcunov wrote:
> > It's not enough to simply disable event on overflow the cpuc->active_mask
> > should be cleared as well otherwise counter may stall in "active" even
> > in real being already disabled (which potentially may lead to the situation
> > that user may not use this counter further).
> 
> I also noticed this patch fixed some unknown NMIs on a P4 when I stressed
> the box
> 
> Acked-by: Don Zickus <dzickus@redhat.com>

Ok. Mind one of you resending all pending patches in this area, with all 
Acked-by's and Tested-by's in place?

I'd like to ask one more thing as well: please submit the patches in a single 
e-mail thread - i.e. with a proper 0/N and e-mail threading, so that anyone can 
see that they are a group of patches. Current submissions are various versions 
of patches with no clear grouping.

Thanks,

	Ingo

Re: [PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Don Zickus]

On Thu, Apr 21, 2011 at 03:17:48PM +0200, Ingo Molnar wrote:
> 
> * Don Zickus <dzickus@redhat.com> wrote:
> 
> > On Wed, Apr 20, 2011 at 07:13:37PM +0400, Cyrill Gorcunov wrote:
> > > It's not enough to simply disable event on overflow the cpuc->active_mask
> > > should be cleared as well otherwise counter may stall in "active" even
> > > in real being already disabled (which potentially may lead to the situation
> > > that user may not use this counter further).
> > 
> > I also noticed this patch fixed some unknown NMIs on a P4 when I stressed
> > the box
> > 
> > Acked-by: Don Zickus <dzickus@redhat.com>
> 
> Ok. Mind one of you resending all pending patches in this area, with all 
> Acked-by's and Tested-by's in place?

Cyril is that something you can do because I think all of these patches
revolve around P4 fixes?  Though I don't know what are all the patches.  I
know I had one and this patch.  Is there another on Cyril?

Cheers,
Don

Re: [PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Cyrill Gorcunov]

On 04/21/2011 05:17 PM, Ingo Molnar wrote:
> 
> * Don Zickus <dzickus@redhat.com> wrote:
> 
>> On Wed, Apr 20, 2011 at 07:13:37PM +0400, Cyrill Gorcunov wrote:
>>> It's not enough to simply disable event on overflow the cpuc->active_mask
>>> should be cleared as well otherwise counter may stall in "active" even
>>> in real being already disabled (which potentially may lead to the situation
>>> that user may not use this counter further).
>>
>> I also noticed this patch fixed some unknown NMIs on a P4 when I stressed
>> the box
>>
>> Acked-by: Don Zickus <dzickus@redhat.com>
> 
> Ok. Mind one of you resending all pending patches in this area, with all 
> Acked-by's and Tested-by's in place?
> 
> I'd like to ask one more thing as well: please submit the patches in a single 
> e-mail thread - i.e. with a proper 0/N and e-mail threading, so that anyone can 
> see that they are a group of patches. Current submissions are various versions 
> of patches with no clear grouping.
> 
> Thanks,
> 
> 	Ingo

Don, will you pick it up in addition to your unknown NMI fast-fix?

-- 
    Cyrill

Re: [PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Cyrill Gorcunov]

On 04/21/2011 05:57 PM, Don Zickus wrote:
...
> 
> Cyril is that something you can do because I think all of these patches
> revolve around P4 fixes?  Though I don't know what are all the patches.  I
> know I had one and this patch.  Is there another on Cyril?
> 
> Cheers,
> Don

Don, I've two patches for P4 and one yours which is a fast fix for unknown-nmi.
So I could gather all three patches and send them to Ingo. And you'll see them
and review again as well. Deal? :)

-- 
    Cyrill

Re: [PATCH -tip] perf, x86: P4 PMU - Don't forget to clear cpuc->active_mask on overflow

[Don Zickus]

On Thu, Apr 21, 2011 at 06:00:18PM +0400, Cyrill Gorcunov wrote:
> On 04/21/2011 05:57 PM, Don Zickus wrote:
> ...
> > 
> > Cyril is that something you can do because I think all of these patches
> > revolve around P4 fixes?  Though I don't know what are all the patches.  I
> > know I had one and this patch.  Is there another on Cyril?
> > 
> > Cheers,
> > Don
> 
> Don, I've two patches for P4 and one yours which is a fast fix for unknown-nmi.
> So I could gather all three patches and send them to Ingo. And you'll see them
> and review again as well. Deal? :)

You can add my acked-by and/or review-by tags to the patches.  But yeah I
can review them again.

Cheers,
Don