From: "Liang, Kan" <kan.liang@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
CC: "mingo@redhat.com" <mingo@redhat.com>,
        "acme@kernel.org" <acme@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "alexander.shishkin@linux.intel.com" 
        <alexander.shishkin@linux.intel.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "namhyung@kernel.org" <namhyung@kernel.org>,
        "jolsa@kernel.org" <jolsa@kernel.org>,
        "Hunter, Adrian" <adrian.hunter@intel.com>,
        "wangnan0@huawei.com" <wangnan0@huawei.com>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "andi@firstfloor.org" <andi@firstfloor.org>
Subject: RE: [PATCH V2 03/13] perf/x86: output sampling overhead
Thread-Topic: [PATCH V2 03/13] perf/x86: output sampling overhead
Thread-Index: AQHSTOHSuMQJFJKXz0+jqt27jlukiaD6RYyAgADDP4D//4M1AIAAhmxQ
Date: Tue, 6 Dec 2016 15:47:40 +0000
Message-ID: <37D7C6CF3E00A74B8858931C1DB2F07750CA9EFB@SHSMSX103.ccr.corp.intel.com>
References: <1480713561-6617-1-git-send-email-kan.liang@intel.com>
 <1480713561-6617-4-git-send-email-kan.liang@intel.com>
 <20161206112013.GJ3124@twins.programming.kicks-ass.net>
 <37D7C6CF3E00A74B8858931C1DB2F07750CA9EAE@SHSMSX103.ccr.corp.intel.com>
 <20161206153222.GB3061@worktop.programming.kicks-ass.net>
In-Reply-To: <20161206153222.GB3061@worktop.programming.kicks-ass.net>
Accept-Language: zh-CN, en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1970
Lines: 58


> 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@intel.com wrote:
> > > > From: Kan Liang <kan.liang@intel.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@intel.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.

Yes, it only generate/log the output. Sorry for the confused wording.

The NMI overhead is pmu specific overhead. So the NMI overhead output
should be generated in pmu code.  
I assume that the pmu:del is the last called pmu function when perf finish.
Is it a good place for logging?

Thanks,
Kan