aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJiri Olsa <jolsa@redhat.com>2017-07-19 03:52:47 -0400
committerIngo Molnar <mingo@kernel.org>2017-07-21 03:58:39 -0400
commitdf6c3db8d30fb1699ccbc403196b86324f4257af (patch)
tree50f70bbeee63d2f4c0ebe3ccea94311a8be8aac0
parent2aeb1883547626d82c597cce2c99f0b9c62e2425 (diff)
perf/x86/intel: Add proper condition to run sched_task callbacks
We have 2 functions using the same sched_task callback: - PEBS drain for free running counters - LBR save/store Both of them are called from intel_pmu_sched_task() and either of them can be unwillingly triggered when the other one is configured to run. Let's say there's PEBS drain configured in sched_task callback for the event, but in the callback itself (intel_pmu_sched_task()) we will also run the code for LBR save/restore, which we did not ask for, but the code in intel_pmu_sched_task() does not check for that. This can lead to extra cycles in some perf monitoring, like when we monitor PEBS event without LBR data. # perf record --no-timestamp -c 10000 -e cycles:p ./perf bench sched pipe -l 1000000 (We need PEBS, non freq/non timestamp event to enable the sched_task callback) The perf stat of cycles and msr:write_msr for above command before the change: ... Performance counter stats for './perf record --no-timestamp -c 10000 -e cycles:p \ ./perf bench sched pipe -l 1000000' (5 runs): 18,519,557,441 cycles:k 91,195,527 msr:write_msr 29.334476406 seconds time elapsed And after the change: ... Performance counter stats for './perf record --no-timestamp -c 10000 -e cycles:p \ ./perf bench sched pipe -l 1000000' (5 runs): 18,704,973,540 cycles:k 27,184,720 msr:write_msr 16.977875900 seconds time elapsed There's no affect on cycles:k because the sched_task happens with events switched off, however the msr:write_msr tracepoint counter together with almost 50% of time speedup show the improvement. Monitoring LBR event and having extra PEBS drain processing in sched_task callback showed just a little speedup, because the drain function does not do much extra work in case there is no PEBS data. Adding conditions to recognize the configured work that needs to be done in the x86_pmu's sched_task callback. Suggested-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Kan Liang <kan.liang@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@kernel.org> Link: http://lkml.kernel.org/r/20170719075247.GA27506@krava Signed-off-by: Ingo Molnar <mingo@kernel.org>
-rw-r--r--arch/x86/events/intel/core.c6
-rw-r--r--arch/x86/events/intel/ds.c14
-rw-r--r--arch/x86/events/intel/lbr.c4
3 files changed, 14 insertions, 10 deletions
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index ede97710c2f4..98b0f0729527 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3397,10 +3397,8 @@ static void intel_pmu_cpu_dying(int cpu)
3397static void intel_pmu_sched_task(struct perf_event_context *ctx, 3397static void intel_pmu_sched_task(struct perf_event_context *ctx,
3398 bool sched_in) 3398 bool sched_in)
3399{ 3399{
3400 if (x86_pmu.pebs_active) 3400 intel_pmu_pebs_sched_task(ctx, sched_in);
3401 intel_pmu_pebs_sched_task(ctx, sched_in); 3401 intel_pmu_lbr_sched_task(ctx, sched_in);
3402 if (x86_pmu.lbr_nr)
3403 intel_pmu_lbr_sched_task(ctx, sched_in);
3404} 3402}
3405 3403
3406PMU_FORMAT_ATTR(offcore_rsp, "config1:0-63"); 3404PMU_FORMAT_ATTR(offcore_rsp, "config1:0-63");
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 6dc8a59e1bfb..a322fed5f8ed 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -606,12 +606,6 @@ static inline void intel_pmu_drain_pebs_buffer(void)
606 x86_pmu.drain_pebs(&regs); 606 x86_pmu.drain_pebs(&regs);
607} 607}
608 608
609void intel_pmu_pebs_sched_task(struct perf_event_context *ctx, bool sched_in)
610{
611 if (!sched_in)
612 intel_pmu_drain_pebs_buffer();
613}
614
615/* 609/*
616 * PEBS 610 * PEBS
617 */ 611 */
@@ -822,6 +816,14 @@ static inline bool pebs_needs_sched_cb(struct cpu_hw_events *cpuc)
822 return cpuc->n_pebs && (cpuc->n_pebs == cpuc->n_large_pebs); 816 return cpuc->n_pebs && (cpuc->n_pebs == cpuc->n_large_pebs);
823} 817}
824 818
819void intel_pmu_pebs_sched_task(struct perf_event_context *ctx, bool sched_in)
820{
821 struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
822
823 if (!sched_in && pebs_needs_sched_cb(cpuc))
824 intel_pmu_drain_pebs_buffer();
825}
826
825static inline void pebs_update_threshold(struct cpu_hw_events *cpuc) 827static inline void pebs_update_threshold(struct cpu_hw_events *cpuc)
826{ 828{
827 struct debug_store *ds = cpuc->ds; 829 struct debug_store *ds = cpuc->ds;
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index eb261656a320..955457a30197 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -380,8 +380,12 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
380 380
381void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in) 381void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in)
382{ 382{
383 struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
383 struct x86_perf_task_context *task_ctx; 384 struct x86_perf_task_context *task_ctx;
384 385
386 if (!cpuc->lbr_users)
387 return;
388
385 /* 389 /*
386 * If LBR callstack feature is enabled and the stack was saved when 390 * If LBR callstack feature is enabled and the stack was saved when
387 * the task was scheduled out, restore the stack. Otherwise flush 391 * the task was scheduled out, restore the stack. Otherwise flush