Messages in this thread Patch in this message | | | Date | Thu, 24 Nov 2016 20:02:42 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH 02/14] perf/x86: output NMI overhead |
| |
On Thu, Nov 24, 2016 at 04:19:09PM +0000, Mark Rutland wrote: > > static void > > event_sched_out(struct perf_event *event, > > struct perf_cpu_context *cpuctx, > > - struct perf_event_context *ctx) > > + struct perf_event_context *ctx, > > + bool log_overhead) > > Boolean parameter are always confusing. Why not pass the flags directly? That > way we can pass *which* overhead to log, and make the callsites easier to > understand. > > > event->tstamp_stopped = tstamp; > > - event->pmu->del(event, 0); > > + event->pmu->del(event, log_overhead ? PERF_EF_LOG : 0); > > ... which we could pass on here. > > > @@ -1835,20 +1835,21 @@ event_sched_out(struct perf_event *event, > > static void > > group_sched_out(struct perf_event *group_event, > > struct perf_cpu_context *cpuctx, > > - struct perf_event_context *ctx) > > + struct perf_event_context *ctx, > > + bool log_overhead) > > Likewise. > > > @@ -1872,7 +1873,7 @@ __perf_remove_from_context(struct perf_event *event, > > { > > unsigned long flags = (unsigned long)info; > > > > - event_sched_out(event, cpuctx, ctx); > > + event_sched_out(event, cpuctx, ctx, false); > > if (flags & DETACH_GROUP) > > perf_group_detach(event); > > list_del_event(event, ctx); > > @@ -1918,9 +1919,9 @@ static void __perf_event_disable(struct perf_event *event, > > update_cgrp_time_from_event(event); > > update_group_times(event); > > if (event == event->group_leader) > > - group_sched_out(event, cpuctx, ctx); > > + group_sched_out(event, cpuctx, ctx, true); > > else > > - event_sched_out(event, cpuctx, ctx); > > + event_sched_out(event, cpuctx, ctx, true); > > Why does this differ from __perf_remove_from_context()?
So I'm not a great fan of sprinkling all this through the core code at all.
| |