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

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 <[email protected]>

---

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
=====================================================================

arch/x86/kernel/cpu/perf_event_p4.c +1 -1

@@ -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) {

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

Quoted 4 lines · 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).

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

Acked-by Don Zickus <[email protected]>

Quoted 6 lines · Don Zickus

Quoted 4 lines · 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).

Quoted 4 lines

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

Acked-by Don Zickus <[email protected]>

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

On 04/21/2011 05:57 PM, Don Zickus wrote:
...

Quoted 5 lines

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?

Quoted 2 lines

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? :)

Quoted 10 lines · Don Zickus

Quoted 4 lines · 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).

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

Acked-by Don Zickus <[email protected]>

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