Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1535221 > unrolled thread

[PATCH V2 03/13] perf/x86: output sampling overhead

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V2 03/13] perf/x86: output sampling overhead kan.liang@intel.com - 2016-12-02 22:30 +0100
    Re: [PATCH V2 03/13] perf/x86: output sampling overhead Peter Zijlstra <peterz@infradead.org> - 2016-12-06 12:30 +0100
      RE: [PATCH V2 03/13] perf/x86: output sampling overhead "Liang, Kan" <kan.liang@intel.com> - 2016-12-06 16:10 +0100
        Re: [PATCH V2 03/13] perf/x86: output sampling overhead Peter Zijlstra <peterz@infradead.org> - 2016-12-06 16:40 +0100
          RE: [PATCH V2 03/13] perf/x86: output sampling overhead "Liang, Kan" <kan.liang@intel.com> - 2016-12-06 16:50 +0100
            Re: [PATCH V2 03/13] perf/x86: output sampling overhead Peter Zijlstra <peterz@infradead.org> - 2016-12-06 19:30 +0100
              RE: [PATCH V2 03/13] perf/x86: output sampling overhead "Liang, Kan" <kan.liang@intel.com> - 2016-12-07 20:10 +0100
    Re: [PATCH V2 03/13] perf/x86: output sampling overhead Peter Zijlstra <peterz@infradead.org> - 2016-12-06 13:10 +0100

#1535221 — [PATCH V2 03/13] perf/x86: output sampling overhead

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) {
 		/*
@@ -1475,11 +1478,21 @@ void perf_events_lapic_init(void)
 	apic_write(APIC_LVTPC, APIC_DM_NMI);
 }
 
+static void
+perf_calculate_nmi_overhead(u64 time)
+{
+	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+
+	cpuc->nmi_overhead.nr++;
+	cpuc->nmi_overhead.time += time;
+}
+
 static int
 perf_event_nmi_handler(unsigned int cmd, struct pt_regs *regs)
 {
 	u64 start_clock;
 	u64 finish_clock;
+	u64 clock;
 	int ret;
 
 	/*
@@ -1492,8 +1505,10 @@ perf_event_nmi_handler(unsigned int cmd, struct pt_regs *regs)
 	start_clock = sched_clock();
 	ret = x86_pmu.handle_irq(regs);
 	finish_clock = sched_clock();
+	clock = finish_clock - start_clock;
 
-	perf_sample_event_took(finish_clock - start_clock);
+	perf_calculate_nmi_overhead(clock);
+	perf_sample_event_took(clock);
 
 	return ret;
 }
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index a77ee02..7a03384 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -230,6 +230,8 @@ struct cpu_hw_events {
 	struct intel_excl_cntrs		*excl_cntrs;
 	int excl_thread_id; /* 0 or 1 */
 
+	struct perf_overhead_entry     nmi_overhead;
+
 	/*
 	 * AMD specific bits
 	 */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index bb0ecf0..fe7b1fb 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -1001,6 +1001,7 @@ enum perf_record_overhead_type {
 	PERF_CORE_OVERHEAD	 = 0,
 
 	PERF_PMU_OVERHEAD	 = 20,
+	PERF_PMU_SAMPLE_OVERHEAD = 20,
 
 	PERF_OVERHEAD_MAX,
 };
-- 
2.5.5

[toc] | [next] | [standalone]

#1536879

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.

[toc] | [prev] | [next] | [standalone]

#1537023

> 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.
The calculation is in nmi_hanlder as below. I will make it clear in the changelog.

@@ -1492,8 +1505,10 @@ perf_event_nmi_handler(unsigned int cmd, struct pt_regs *regs)
 	start_clock = sched_clock();
 	ret = x86_pmu.handle_irq(regs);
 	finish_clock = sched_clock();
+	clock = finish_clock - start_clock;
 
-	perf_sample_event_took(finish_clock - start_clock);
+	perf_calculate_nmi_overhead(clock);
+	perf_sample_event_took(clock);
 
 	return ret;
 }

Thanks,
Kan

[toc] | [prev] | [next] | [standalone]

#1537046

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.

[toc] | [prev] | [next] | [standalone]

#1537059

> 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

[toc] | [prev] | [next] | [standalone]

#1537193

On Tue, Dec 06, 2016 at 03:47:40PM +0000, Liang, Kan wrote:

> > 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.

True, but you're also accounting in a per-cpu bucket, which means it
includes all events. At which point the per-event overhead thing doesn't
really make sense.

It also means that previous sessions influence the numbers of our
current session; there's no explicit reset of the numbers.

> I assume that the pmu:del is the last called pmu function when perf finish.
> Is it a good place for logging?

No, its horrible. Sure, we'll call pmu::del on events, but yuck.

You really only want _one_ invocation when you stop using the event, and
we don't really have a good place for that. But instead of creating one,
you do horrible things.

Now, I realize there's a bit of a catch-22 in that the moment we know
the event is going away, its already gone from userspace. So we cannot
dump data from there in general..

Howver, if we have output redirection we can, but that would make things
depend on that and it cannot be used for the last event who's buffer
we're using.

Another option would be to introduce PERF_EVENT_IOC_STAT or something
like that, and have the tool call that when its 'done'.

[toc] | [prev] | [next] | [standalone]

#1538008

> On Tue, Dec 06, 2016 at 03:47:40PM +0000, Liang, Kan wrote:
> 
> > > 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.
> 
> True, but you're also accounting in a per-cpu bucket, which means it
> includes all events. At which point the per-event overhead thing doesn't
> really make sense.
> 
> It also means that previous sessions influence the numbers of our current
> session; there's no explicit reset of the numbers.
> 
> > I assume that the pmu:del is the last called pmu function when perf finish.
> > Is it a good place for logging?
> 
> No, its horrible. Sure, we'll call pmu::del on events, but yuck.
> 
> You really only want _one_ invocation when you stop using the event, and
> we don't really have a good place for that. But instead of creating one, you
> do horrible things.
> 
> Now, I realize there's a bit of a catch-22 in that the moment we know the
> event is going away, its already gone from userspace. So we cannot dump
> data from there in general..
> 
> Howver, if we have output redirection we can, but that would make things
> depend on that and it cannot be used for the last event who's buffer we're
> using.
> 
> Another option would be to introduce PERF_EVENT_IOC_STAT or something
> like that, and have the tool call that when its 'done'.
> 

OK. I think I will implement a new ioctl PERF_EVENT_IOC_STAT.

The IOC_STAT will be called by tool when its 'start' and 'done'.
I will also introduce two new ioc flags.
(PERF_IOC_FLAG_STAT_START and PERF_IOC_FLAG_STAT_DONE)
In 'start', the kernel will reset the numbers.
In 'done', the kernel will generate all outputs. The overhead numbers are from
different cpu. To distinguish them,  we have to add cpu in overhead_entry. We
cannot trust sample_id.
struct perf_overhead_entry {
	__u32	cpu;
	__u32	nr;
	__u64	time;
};

I will also add void (*overhead_stat) in struct pmu to do pmu specific reset and
generation.

In V2, the three overheads are stored in different per-event/per-cpu ctx.
For next V3, I will store all the overheads in pmu's cpuctx.
So the number will be the overhead for pmu, not the global system. 
It should be more clear and useful. 

how does it sound?

Thanks,
Kan 

[toc] | [prev] | [next] | [standalone]

#1536893

On Fri, Dec 02, 2016 at 04:19:11PM -0500, kan.liang@intel.com wrote:
> +static void
> +perf_calculate_nmi_overhead(u64 time)
> +{
> +	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +
> +	cpuc->nmi_overhead.nr++;
> +	cpuc->nmi_overhead.time += time;
> +}

This function doesn't calculate anything much.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web