Re: [PATCH 09/12] perf_events: add hook to flush branch_stack on context switch (v2)

[Peter Zijlstra <peterz@infradead.org>]

On Wed, 2011-12-07 at 10:25 -0800, Stephane Eranian wrote:
> On Mon, Dec 5, 2011 at 1:37 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Fri, 2011-10-14 at 14:37 +0200, Stephane Eranian wrote:
> >> +               /*
> >> +                * check if the context has at least one
> >> +                * event using PERF_SAMPLE_BRANCH_STACK
> >> +                */
> >> +               if (cpuctx->ctx.nr_branch_stack > 0
> >> +                   && pmu->flush_branch_stack) {
> >> +
> >> +                       pmu = cpuctx->ctx.pmu;
> >> +
> >> +                       perf_ctx_lock(cpuctx, cpuctx->task_ctx);
> >> +
> >> +                       perf_pmu_disable(pmu);
> >> +
> >> +                       pmu->flush_branch_stack();
> >> +
> >> +                       perf_pmu_enable(pmu);
> >> +
> >> +                       perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
> >> +               }
> >> +       }
> >
> > (what whitespace looks funny)
> >
> > So all PMUs not supporting this branch stuff will fail to create a
> > has_branch_stack() event, right? Thus all ctx with !0 nr_branch_stack
> > support it. Doesn't this make the test for pmu->flush_branch_stack
> > redundant?
> >
> >
> No, nr_branch_stack counts the number of active events with
> branch_stack. It's like the ctx->nr_cgroups. Processors which
> do not support branch_stack will always have this field to 0.
> It's not because a processor supports branch_stack that we
> need to call flush_branch_stack(), i.e., we use a lazy approach.

What you're saying is we can support branch stack and not need
flush_branch_stack()? Say in the case the x86 LBR TOS field would be a
full u64 counter, since then we could sample the TOS on context switch
and filter on that, obviating the hard reset we do now.

And the advantage of testing for the operation as opposed to putting in
a dummy function (like we do for most other optional methods) is
avoiding all that ctx_lock and pmu_disable muck.

Fair enough.