linux-kernel - Re: [PATCH 09/12] perf_events: add hook to flush branch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1323341376.17673.10.camel@twins>
Date:	Thu, 08 Dec 2011 11:49:36 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Stephane Eranian <eranian@...gle.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, acme@...hat.com,
	ming.m.lin@...el.com, andi@...stfloor.org, robert.richter@....com,
	ravitillo@....gov, will.deacon@....com, paulus@...ba.org,
	benh@...nel.crashing.org, rth@...ddle.net, ralf@...ux-mips.org,
	davem@...emloft.net, lethal@...ux-sh.org
Subject: Re: [PATCH 09/12] perf_events: add hook to flush branch_stack on
 context switch (v2)

On Wed, 2011-12-07 at 10:25 -0800, Stephane Eranian wrote:
> On Mon, Dec 5, 2011 at 1:37 PM, Peter Zijlstra <peterz@...radead.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/