linux-kernel - Re: [PATCH V2 03/13] perf/x86: output sampling overhead

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:   Tue, 6 Dec 2016 16:32:22 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Liang, Kan" <kan.liang@...el.com>
Cc:     "mingo@...hat.com" <mingo@...hat.com>,
        "acme@...nel.org" <acme@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "alexander.shishkin@...ux.intel.com" 
        <alexander.shishkin@...ux.intel.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "namhyung@...nel.org" <namhyung@...nel.org>,
        "jolsa@...nel.org" <jolsa@...nel.org>,
        "Hunter, Adrian" <adrian.hunter@...el.com>,
        "wangnan0@...wei.com" <wangnan0@...wei.com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "andi@...stfloor.org" <andi@...stfloor.org>
Subject: Re: [PATCH V2 03/13] perf/x86: output sampling overhead

On Tue, Dec 06, 2016 at 03:02:20PM +0000, Liang, Kan wrote:
> 
> 
> > On Fri, Dec 02, 2016 at 04:19:11PM -0500, kan.liang@...el.com wrote:
> > > From: Kan Liang <kan.liang@...el.com>
> > >
> > > On x86, NMI handler is the most important part which brings overhead
> > > for sampling. Adding a pmu specific overhead type
> > > PERF_PMU_SAMPLE_OVERHEAD for it.
> > >
> > > For other architectures which may don't have NMI, the overhead type
> > > can be reused.
> > >
> > > Signed-off-by: Kan Liang <kan.liang@...el.com>
> > > ---
> > >  arch/x86/events/core.c          | 17 ++++++++++++++++-
> > >  arch/x86/events/perf_event.h    |  2 ++
> > >  include/uapi/linux/perf_event.h |  1 +
> > >  3 files changed, 19 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index
> > > 9d4bf3a..de40f96 100644
> > > --- a/arch/x86/events/core.c
> > > +++ b/arch/x86/events/core.c
> > > @@ -1397,6 +1397,9 @@ static void x86_pmu_del(struct perf_event
> > > *event, int flags)
> > >
> > >  	perf_event_update_userpage(event);
> > >
> > > +	if ((flags & PERF_EF_LOG) && cpuc->nmi_overhead.nr)
> > > +		perf_log_overhead(event, PERF_PMU_SAMPLE_OVERHEAD,
> > > +&cpuc->nmi_overhead);
> > > +
> > >  do_del:
> > >  	if (x86_pmu.del) {
> > >  		/*
> > 
> > That's not at all mentioned in the changelog, and it clearly isn't
> > nmi_overhead.
> 
> Here it only records the overhead, not calculate.

It doesn't record anything, it generates the output. And it doesn't
explain why that needs to be in pmu::del(), in general that's a horrible
thing to do.