On Tue, Oct 20, 2015 at 03:53:02PM -0700, Alexei Starovoitov wrote: > On 10/20/15 12:22 AM, Kaixu Xia wrote: > >diff --git a/kernel/events/core.c b/kernel/events/core.c > >index b11756f..5219635 100644 > >--- a/kernel/events/core.c > >+++ b/kernel/events/core.c > >@@ -6337,6 +6337,9 @@ static int __perf_event_overflow(struct perf_event > >*event, > > irq_work_queue(&event->pending); > > } > > > >+ if (unlikely(!atomic_read(&event->soft_enable))) > >+ return 0; > >+ > > if (event->overflow_handler) > > event->overflow_handler(event, data, regs); > > else > > Peter, > does this part look right or it should be moved right after > if (unlikely(!is_sampling_event(event))) > return 0; > or even to other function? > > It feels to me that it should be moved, since we probably don't > want to active throttling, period adjust and event_limit for events > that are in soft_disabled state.
Depends on what its meant to do. As long as you let the interrupt happen, I think we should in fact do those things (maybe not the event_limit), but period adjustment and esp. throttling are important when the event is enabled. If you want to actually disable the event: pmu->stop() will make it stop, and you can restart using pmu->start(). And I suppose you can wrap that with a counter if you need nesting. I'm not sure if any of that is a viable solution, because the patch description is somewhat short on the problem statement. As is, I'm not too charmed with the patch, but lacking a better understanding of what exactly we're trying to achieve I'm struggling with proposing alternatives. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
