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]
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ