From 00a2916f7f82c348a2a94dbb572874173bc308a3 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 27 Jul 2015 10:35:07 +0200 Subject: perf: Fix running time accounting A recent fix to the shadow timestamp inadvertly broke the running time accounting. We must not update the running timestamp if we fail to schedule the event, the event will not have ran. This can (and did) result in negative total runtime because the stopped timestamp was before the running timestamp (we 'started' but never stopped the event -- because it never really started we didn't have to stop it either). Reported-and-Tested-by: Vince Weaver Fixes: 72f669c0086f ("perf: Update shadow timestamp before add event") Signed-off-by: Peter Zijlstra (Intel) Cc: stable@vger.kernel.org # 4.1 Cc: Shaohua Li Signed-off-by: Thomas Gleixner --- kernel/events/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index d3dae3419b99..10d076b2572c 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1868,8 +1868,6 @@ event_sched_in(struct perf_event *event, perf_pmu_disable(event->pmu); - event->tstamp_running += tstamp - event->tstamp_stopped; - perf_set_shadow_time(event, ctx, tstamp); perf_log_itrace_start(event); @@ -1881,6 +1879,8 @@ event_sched_in(struct perf_event *event, goto out; } + event->tstamp_running += tstamp - event->tstamp_stopped; + if (!is_software_event(event)) cpuctx->active_oncpu++; if (!ctx->nr_active++) -- cgit v1.2.3 From 5497628576a3c5f3dbab224fa5a5d027f43d8b50 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 27 Jul 2015 16:24:51 -0700 Subject: perf stat: Fix transaction lenght metrics The transaction length metrics in perf stat -T broke recently. It would not match the metric correctly and always print K/sec. This was caused by a incorrect update of the cycles_in_tx statistics. Update the correct variable. Also the check for zero division was reversed, which resulted in K/sec being printed for no transactions. Fix this also up. Signed-off-by: Andi Kleen Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/1438039491-22091-1-git-send-email-andi@firstfloor.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/stat-shadow.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c index 53e8bb7bc852..2a5d8d7698ae 100644 --- a/tools/perf/util/stat-shadow.c +++ b/tools/perf/util/stat-shadow.c @@ -85,7 +85,7 @@ void perf_stat__update_shadow_stats(struct perf_evsel *counter, u64 *count, else if (perf_evsel__match(counter, HARDWARE, HW_CPU_CYCLES)) update_stats(&runtime_cycles_stats[ctx][cpu], count[0]); else if (perf_stat_evsel__is(counter, CYCLES_IN_TX)) - update_stats(&runtime_transaction_stats[ctx][cpu], count[0]); + update_stats(&runtime_cycles_in_tx_stats[ctx][cpu], count[0]); else if (perf_stat_evsel__is(counter, TRANSACTION_START)) update_stats(&runtime_transaction_stats[ctx][cpu], count[0]); else if (perf_stat_evsel__is(counter, ELISION_START)) @@ -398,20 +398,18 @@ void perf_stat__print_shadow_stats(FILE *out, struct perf_evsel *evsel, " # %5.2f%% aborted cycles ", 100.0 * ((total2-avg) / total)); } else if (perf_stat_evsel__is(evsel, TRANSACTION_START) && - avg > 0 && runtime_cycles_in_tx_stats[ctx][cpu].n != 0) { total = avg_stats(&runtime_cycles_in_tx_stats[ctx][cpu]); - if (total) + if (avg) ratio = total / avg; fprintf(out, " # %8.0f cycles / transaction ", ratio); } else if (perf_stat_evsel__is(evsel, ELISION_START) && - avg > 0 && runtime_cycles_in_tx_stats[ctx][cpu].n != 0) { total = avg_stats(&runtime_cycles_in_tx_stats[ctx][cpu]); - if (total) + if (avg) ratio = total / avg; fprintf(out, " # %8.0f cycles / elision ", ratio); -- cgit v1.2.3 From 0927beeca5f9d1a7978f8da9c9d28647859816d3 Mon Sep 17 00:00:00 2001 From: Pawel Moll Date: Tue, 28 Jul 2015 15:10:13 +0100 Subject: perf tools: Fix test build error when bindir contains double slash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with a prefix ending with a slash, for example: $ make prefix=/usr/local/ one of the perf tests fail to compile due to BUILD_STR macro mishandling bindir_SQ string containing with two slashes: -DBINDIR="BUILD_STR(/usr/local//bin)" with the following error: CC tests/attr.o tests/attr.c: In function ‘test__attr’: tests/attr.c:168:50: error: expected ‘)’ before ‘;’ token snprintf(path_perf, PATH_MAX, "%s/perf", BINDIR); ^ tests/attr.c:176:1: error: expected ‘;’ before ‘}’ token } ^ tests/attr.c:176:1: error: control reaches end of non-void function [-Werror=return-type] } ^ cc1: all warnings being treated as errors This patch works around the problem by "cleaning" the bindir string using make's abspath function. Signed-off-by: Pawel Moll Acked-by: Jiri Olsa Cc: Adrian Hunter Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1438092613-21014-1-git-send-email-pawel.moll@arm.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 094ddaee104c..d31fac19c30b 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -638,7 +638,7 @@ ifndef DESTDIR prefix ?= $(HOME) endif bindir_relative = bin -bindir = $(prefix)/$(bindir_relative) +bindir = $(abspath $(prefix)/$(bindir_relative)) mandir = share/man infodir = share/info perfexecdir = libexec/perf-core -- cgit v1.2.3 From fed66e2cdd4f127a43fd11b8d92a99bdd429528c Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Thu, 11 Jun 2015 10:32:01 +0200 Subject: perf: Fix fasync handling on inherited events Vince reported that the fasync signal stuff doesn't work proper for inherited events. So fix that. Installing fasync allocates memory and sets filp->f_flags |= FASYNC, which upon the demise of the file descriptor ensures the allocation is freed and state is updated. Now for perf, we can have the events stick around for a while after the original FD is dead because of references from child events. So we cannot copy the fasync pointer around. We can however consistently use the parent's fasync, as that will be updated. Reported-and-Tested-by: Vince Weaver Signed-off-by: Peter Zijlstra (Intel) Cc: Cc: Arnaldo Carvalho deMelo Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: eranian@google.com Link: http://lkml.kernel.org/r/1434011521.1495.71.camel@twins Signed-off-by: Ingo Molnar --- kernel/events/core.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index 10d076b2572c..072b8a686517 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -4740,12 +4740,20 @@ static const struct file_operations perf_fops = { * to user-space before waking everybody up. */ +static inline struct fasync_struct **perf_event_fasync(struct perf_event *event) +{ + /* only the parent has fasync state */ + if (event->parent) + event = event->parent; + return &event->fasync; +} + void perf_event_wakeup(struct perf_event *event) { ring_buffer_wakeup(event); if (event->pending_kill) { - kill_fasync(&event->fasync, SIGIO, event->pending_kill); + kill_fasync(perf_event_fasync(event), SIGIO, event->pending_kill); event->pending_kill = 0; } } @@ -6124,7 +6132,7 @@ static int __perf_event_overflow(struct perf_event *event, else perf_event_output(event, data, regs); - if (event->fasync && event->pending_kill) { + if (*perf_event_fasync(event) && event->pending_kill) { event->pending_wakeup = 1; irq_work_queue(&event->pending); } -- cgit v1.2.3 From ee9397a6fb9bc4e52677f5e33eed4abee0f515e6 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 27 Jul 2015 00:31:08 +0100 Subject: perf: Fix double-free of the AUX buffer If rb->aux_refcount is decremented to zero before rb->refcount, __rb_free_aux() may be called twice resulting in a double free of rb->aux_pages. Fix this by adding a check to __rb_free_aux(). Signed-off-by: Ben Hutchings Signed-off-by: Peter Zijlstra (Intel) Cc: Alexander Shishkin Cc: Arnaldo Carvalho de Melo Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: stable@vger.kernel.org Fixes: 57ffc5ca679f ("perf: Fix AUX buffer refcounting") Link: http://lkml.kernel.org/r/1437953468.12842.17.camel@decadent.org.uk Signed-off-by: Ingo Molnar --- kernel/events/ring_buffer.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c index b2be01b1aa9d..c8aa3f75bc4d 100644 --- a/kernel/events/ring_buffer.c +++ b/kernel/events/ring_buffer.c @@ -559,11 +559,13 @@ static void __rb_free_aux(struct ring_buffer *rb) rb->aux_priv = NULL; } - for (pg = 0; pg < rb->aux_nr_pages; pg++) - rb_free_aux_page(rb, pg); + if (rb->aux_nr_pages) { + for (pg = 0; pg < rb->aux_nr_pages; pg++) + rb_free_aux_page(rb, pg); - kfree(rb->aux_pages); - rb->aux_nr_pages = 0; + kfree(rb->aux_pages); + rb->aux_nr_pages = 0; + } } void rb_free_aux(struct ring_buffer *rb) -- cgit v1.2.3 From c7999c6f3fed9e383d3131474588f282ae6d56b9 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 4 Aug 2015 19:22:49 +0200 Subject: perf: Fix PERF_EVENT_IOC_PERIOD migration race I ran the perf fuzzer, which triggered some WARN()s which are due to trying to stop/restart an event on the wrong CPU. Use the normal IPI pattern to ensure we run the code on the correct CPU. Signed-off-by: Peter Zijlstra (Intel) Cc: Vince Weaver Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Fixes: bad7192b842c ("perf: Fix PERF_EVENT_IOC_PERIOD to force-reset the period") Signed-off-by: Ingo Molnar --- kernel/events/core.c | 75 ++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index 072b8a686517..e6feb5114134 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -3958,28 +3958,21 @@ static void perf_event_for_each(struct perf_event *event, perf_event_for_each_child(sibling, func); } -static int perf_event_period(struct perf_event *event, u64 __user *arg) -{ - struct perf_event_context *ctx = event->ctx; - int ret = 0, active; +struct period_event { + struct perf_event *event; u64 value; +}; - if (!is_sampling_event(event)) - return -EINVAL; - - if (copy_from_user(&value, arg, sizeof(value))) - return -EFAULT; - - if (!value) - return -EINVAL; +static int __perf_event_period(void *info) +{ + struct period_event *pe = info; + struct perf_event *event = pe->event; + struct perf_event_context *ctx = event->ctx; + u64 value = pe->value; + bool active; - raw_spin_lock_irq(&ctx->lock); + raw_spin_lock(&ctx->lock); if (event->attr.freq) { - if (value > sysctl_perf_event_sample_rate) { - ret = -EINVAL; - goto unlock; - } - event->attr.sample_freq = value; } else { event->attr.sample_period = value; @@ -3998,11 +3991,53 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg) event->pmu->start(event, PERF_EF_RELOAD); perf_pmu_enable(ctx->pmu); } + raw_spin_unlock(&ctx->lock); -unlock: + return 0; +} + +static int perf_event_period(struct perf_event *event, u64 __user *arg) +{ + struct period_event pe = { .event = event, }; + struct perf_event_context *ctx = event->ctx; + struct task_struct *task; + u64 value; + + if (!is_sampling_event(event)) + return -EINVAL; + + if (copy_from_user(&value, arg, sizeof(value))) + return -EFAULT; + + if (!value) + return -EINVAL; + + if (event->attr.freq && value > sysctl_perf_event_sample_rate) + return -EINVAL; + + task = ctx->task; + pe.value = value; + + if (!task) { + cpu_function_call(event->cpu, __perf_event_period, &pe); + return 0; + } + +retry: + if (!task_function_call(task, __perf_event_period, &pe)) + return 0; + + raw_spin_lock_irq(&ctx->lock); + if (ctx->is_active) { + raw_spin_unlock_irq(&ctx->lock); + task = ctx->task; + goto retry; + } + + __perf_event_period(&pe); raw_spin_unlock_irq(&ctx->lock); - return ret; + return 0; } static const struct file_operations perf_fops; -- cgit v1.2.3 From dbc72b7a0c673ff00fdeb21d3a26064e2185baf4 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 10 Aug 2015 14:17:34 +0200 Subject: perf/x86/intel: Fix memory leak on hot-plug allocation fail We fail to free the shared_regs allocation if the constraint_list allocation fails. Cure this and be more consistent in NULL-ing the pointers after free. Signed-off-by: Peter Zijlstra (Intel) Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Thomas Gleixner Signed-off-by: Ingo Molnar --- arch/x86/kernel/cpu/perf_event_intel.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c index b9826a981fb2..6326ae24e4d5 100644 --- a/arch/x86/kernel/cpu/perf_event_intel.c +++ b/arch/x86/kernel/cpu/perf_event_intel.c @@ -2534,7 +2534,7 @@ static int intel_pmu_cpu_prepare(int cpu) if (x86_pmu.extra_regs || x86_pmu.lbr_sel_map) { cpuc->shared_regs = allocate_shared_regs(cpu); if (!cpuc->shared_regs) - return NOTIFY_BAD; + goto err; } if (x86_pmu.flags & PMU_FL_EXCL_CNTRS) { @@ -2542,18 +2542,27 @@ static int intel_pmu_cpu_prepare(int cpu) cpuc->constraint_list = kzalloc(sz, GFP_KERNEL); if (!cpuc->constraint_list) - return NOTIFY_BAD; + goto err_shared_regs; cpuc->excl_cntrs = allocate_excl_cntrs(cpu); - if (!cpuc->excl_cntrs) { - kfree(cpuc->constraint_list); - kfree(cpuc->shared_regs); - return NOTIFY_BAD; - } + if (!cpuc->excl_cntrs) + goto err_constraint_list; + cpuc->excl_thread_id = 0; } return NOTIFY_OK; + +err_constraint_list: + kfree(cpuc->constraint_list); + cpuc->constraint_list = NULL; + +err_shared_regs: + kfree(cpuc->shared_regs); + cpuc->shared_regs = NULL; + +err: + return NOTIFY_BAD; } static void intel_pmu_cpu_starting(int cpu) -- cgit v1.2.3 From d7a702f0b1033cf402fef65bd6395072738f0844 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 6 Aug 2015 13:12:43 +0100 Subject: perf/x86/intel/cqm: Do not access cpu_data() from CPU_UP_PREPARE handler Tony reports that booting his 144-cpu machine with maxcpus=10 triggers the following WARN_ON(): [ 21.045727] WARNING: CPU: 8 PID: 647 at arch/x86/kernel/cpu/perf_event_intel_cqm.c:1267 intel_cqm_cpu_prepare+0x75/0x90() [ 21.045744] CPU: 8 PID: 647 Comm: systemd-udevd Not tainted 4.2.0-rc4 #1 [ 21.045745] Hardware name: Intel Corporation BRICKLAND/BRICKLAND, BIOS BRHSXSD1.86B.0066.R00.1506021730 06/02/2015 [ 21.045747] 0000000000000000 0000000082771b09 ffff880856333ba8 ffffffff81669b67 [ 21.045748] 0000000000000000 0000000000000000 ffff880856333be8 ffffffff8107b02a [ 21.045750] ffff88085b789800 ffff88085f68a020 ffffffff819e2470 000000000000000a [ 21.045750] Call Trace: [ 21.045757] [] dump_stack+0x45/0x57 [ 21.045759] [] warn_slowpath_common+0x8a/0xc0 [ 21.045761] [] warn_slowpath_null+0x1a/0x20 [ 21.045762] [] intel_cqm_cpu_prepare+0x75/0x90 [ 21.045764] [] intel_cqm_cpu_notifier+0x42/0x160 [ 21.045767] [] notifier_call_chain+0x4d/0x80 [ 21.045769] [] __raw_notifier_call_chain+0xe/0x10 [ 21.045770] [] _cpu_up+0xe8/0x190 [ 21.045771] [] cpu_up+0x7a/0xa0 [ 21.045774] [] cpu_subsys_online+0x40/0x90 [ 21.045777] [] device_online+0x67/0x90 [ 21.045778] [] online_store+0x8a/0xa0 [ 21.045782] [] dev_attr_store+0x18/0x30 [ 21.045785] [] sysfs_kf_write+0x3a/0x50 [ 21.045786] [] kernfs_fop_write+0x120/0x170 [ 21.045789] [] __vfs_write+0x37/0x100 [ 21.045791] [] ? __sb_start_write+0x58/0x110 [ 21.045795] [] ? security_file_permission+0x3d/0xc0 [ 21.045796] [] vfs_write+0xa9/0x190 [ 21.045797] [] SyS_write+0x55/0xc0 [ 21.045800] [] ? do_page_fault+0x30/0x80 [ 21.045804] [] entry_SYSCALL_64_fastpath+0x12/0x71 [ 21.045805] ---[ end trace fe228b836d8af405 ]--- The root cause is that CPU_UP_PREPARE is completely the wrong notifier action from which to access cpu_data(), because smp_store_cpu_info() won't have been executed by the target CPU at that point, which in turn means that ->x86_cache_max_rmid and ->x86_cache_occ_scale haven't been filled out. Instead let's invoke our handler from CPU_STARTING and rename it appropriately. Reported-by: Tony Luck Signed-off-by: Matt Fleming Signed-off-by: Peter Zijlstra (Intel) Cc: Ashok Raj Cc: Kanaka Juvva Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Vikas Shivappa Link: http://lkml.kernel.org/r/1438863163-14083-1-git-send-email-matt@codeblueprint.co.uk Signed-off-by: Ingo Molnar --- arch/x86/kernel/cpu/perf_event_intel_cqm.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/x86/kernel/cpu/perf_event_intel_cqm.c b/arch/x86/kernel/cpu/perf_event_intel_cqm.c index 63eb68b73589..377e8f8ed391 100644 --- a/arch/x86/kernel/cpu/perf_event_intel_cqm.c +++ b/arch/x86/kernel/cpu/perf_event_intel_cqm.c @@ -1255,7 +1255,7 @@ static inline void cqm_pick_event_reader(int cpu) cpumask_set_cpu(cpu, &cqm_cpumask); } -static void intel_cqm_cpu_prepare(unsigned int cpu) +static void intel_cqm_cpu_starting(unsigned int cpu) { struct intel_pqr_state *state = &per_cpu(pqr_state, cpu); struct cpuinfo_x86 *c = &cpu_data(cpu); @@ -1296,13 +1296,11 @@ static int intel_cqm_cpu_notifier(struct notifier_block *nb, unsigned int cpu = (unsigned long)hcpu; switch (action & ~CPU_TASKS_FROZEN) { - case CPU_UP_PREPARE: - intel_cqm_cpu_prepare(cpu); - break; case CPU_DOWN_PREPARE: intel_cqm_cpu_exit(cpu); break; case CPU_STARTING: + intel_cqm_cpu_starting(cpu); cqm_pick_event_reader(cpu); break; } @@ -1373,7 +1371,7 @@ static int __init intel_cqm_init(void) goto out; for_each_online_cpu(i) { - intel_cqm_cpu_prepare(i); + intel_cqm_cpu_starting(i); cqm_pick_event_reader(i); } -- cgit v1.2.3