Received: by 10.223.185.111 with SMTP id b44csp74098wrg; Fri, 9 Mar 2018 01:10:39 -0800 (PST) X-Google-Smtp-Source: AG47ELvG0dwYecWBqV95HhY00zI82dqk1NcEIb8sl71l41krMzTpFamrqC93zTaviw1shMPlgqdV X-Received: by 10.98.211.198 with SMTP id z67mr29813298pfk.0.1520586639515; Fri, 09 Mar 2018 01:10:39 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1520586639; cv=none; d=google.com; s=arc-20160816; b=SsNExhY3J5W/xycJ6u3eOFryypODRxYy9QSn77vV+Rj06FTXiglVWJbm/w56UCw6M9 thQY38BrK0mnmQXh4Hgfo/5AI8u1esbndYm6JWQSXJpzxChyOFHcywxmpKDkR0ax93Wb 8Nv87U57lfcxbgO2g6IQGPlv9KyqP28SAU5sTZ3tW4NXJYFLSjtdR+9QWnNwc+MoOi1o kwgJXqoKfa8ngy0HpTwax0oXFAwRYYAEJWQLC8JyNoh0j/64uhrIxJqXhnqlixbkPvqM HTfelERCxWJRYUeVNpPU8U9gezL7TnvST+aPJC/VVYsfEc3s7tuPcFtMM9wucWbOepx2 CemQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-disposition :content-transfer-encoding:mime-version:robot-unsubscribe:robot-id :git-commit-id:subject:to:references:in-reply-to:reply-to:cc :message-id:from:date:arc-authentication-results; bh=bdBBEIxrU+UY9s1rre7w8BxF68NXzTWxovJHpSU05go=; b=HsMLjlKKMB72XyWd2cw0/DlANjUjclUdISs4bwbRgbKD+fiCmmKpP9pVD5fM8bvYNL UthZOI1iVEIwAsj69/pc06BF1bsuaPYY12Sogm2YwuwHg3OdKh3Cs70H5irH3/hf9IdX pEU4cSf5pL3Ie+tNWRMoF2pt6Y9cR/NChrUulNY3ukz3naviSAu3hi6P9i/pdIAXiBRR RqcpVS3tIojV37u+RVeWfztCVhZ3jyI7sJqF5eEvHnsGFdSrUqvEinS/fa9E3DyOtEmQ n77BZF7eIRMlDTz529r5vFtkO1Fna2xvGty30J0CN28QIPNFGOJhBX/5FElLix+hkGyv NT+g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 70-v6si504763pla.64.2018.03.09.01.10.25; Fri, 09 Mar 2018 01:10:39 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751347AbeCIJJM (ORCPT + 99 others); Fri, 9 Mar 2018 04:09:12 -0500 Received: from terminus.zytor.com ([198.137.202.136]:34789 "EHLO terminus.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750965AbeCIJJJ (ORCPT ); Fri, 9 Mar 2018 04:09:09 -0500 Received: from terminus.zytor.com (localhost [127.0.0.1]) by terminus.zytor.com (8.15.2/8.15.2) with ESMTP id w2998jNn031576; Fri, 9 Mar 2018 01:08:45 -0800 Received: (from tipbot@localhost) by terminus.zytor.com (8.15.2/8.15.2/Submit) id w2998jIG031573; Fri, 9 Mar 2018 01:08:45 -0800 Date: Fri, 9 Mar 2018 01:08:45 -0800 X-Authentication-Warning: terminus.zytor.com: tipbot set sender to tipbot@zytor.com using -f From: tip-bot for Kan Liang Message-ID: Cc: acme@redhat.com, torvalds@linux-foundation.org, peterz@infradead.org, jolsa@redhat.com, vincent.weaver@maine.edu, mingo@kernel.org, kan.liang@linux.intel.com, alexander.shishkin@linux.intel.com, hpa@zytor.com, linux-kernel@vger.kernel.org, eranian@google.com, tglx@linutronix.de Reply-To: tglx@linutronix.de, eranian@google.com, linux-kernel@vger.kernel.org, hpa@zytor.com, alexander.shishkin@linux.intel.com, kan.liang@linux.intel.com, mingo@kernel.org, vincent.weaver@maine.edu, jolsa@redhat.com, peterz@infradead.org, torvalds@linux-foundation.org, acme@redhat.com In-Reply-To: <1518474035-21006-2-git-send-email-kan.liang@linux.intel.com> References: <1518474035-21006-2-git-send-email-kan.liang@linux.intel.com> To: linux-tip-commits@vger.kernel.org Subject: [tip:perf/core] perf/x86/intel: Fix event update for auto-reload Git-Commit-ID: d31fc13fdcb20e1c317f9a7dd6273c18fbd58308 X-Mailer: tip-git-log-daemon Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on terminus.zytor.com Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit-ID: d31fc13fdcb20e1c317f9a7dd6273c18fbd58308 Gitweb: https://git.kernel.org/tip/d31fc13fdcb20e1c317f9a7dd6273c18fbd58308 Author: Kan Liang AuthorDate: Mon, 12 Feb 2018 14:20:31 -0800 Committer: Ingo Molnar CommitDate: Fri, 9 Mar 2018 08:22:19 +0100 perf/x86/intel: Fix event update for auto-reload There is a bug when reading event->count with large PEBS enabled. Here is an example: # ./read_count 0x71f0 0x122c0 0x1000000001c54 0x100000001257d 0x200000000bdc5 In fixed period mode, the auto-reload mechanism could be enabled for PEBS events, but the calculation of event->count does not take the auto-reload values into account. Anyone who reads event->count will get the wrong result, e.g x86_pmu_read(). This bug was introduced with the auto-reload mechanism enabled since commit: 851559e35fd5 ("perf/x86/intel: Use the PEBS auto reload mechanism when possible") Introduce intel_pmu_save_and_restart_reload() to calculate the event->count only for auto-reload. Since the counter increments a negative counter value and overflows on the sign switch, giving the interval: [-period, 0] the difference between two consequtive reads is: A) value2 - value1; when no overflows have happened in between, B) (0 - value1) + (value2 - (-period)); when one overflow happened in between, C) (0 - value1) + (n - 1) * (period) + (value2 - (-period)); when @n overflows happened in between. Here A) is the obvious difference, B) is the extension to the discrete interval, where the first term is to the top of the interval and the second term is from the bottom of the next interval and C) the extension to multiple intervals, where the middle term is the whole intervals covered. The equation for all cases is: value2 - value1 + n * period Previously the event->count is updated right before the sample output. But for case A, there is no PEBS record ready. It needs to be specially handled. Remove the auto-reload code from x86_perf_event_set_period() since we'll not longer call that function in this case. Based-on-code-from: Peter Zijlstra (Intel) Signed-off-by: Kan Liang Signed-off-by: Peter Zijlstra (Intel) Cc: Alexander Shishkin Cc: Arnaldo Carvalho de Melo Cc: Jiri Olsa Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Thomas Gleixner Cc: Vince Weaver Cc: acme@kernel.org Fixes: 851559e35fd5 ("perf/x86/intel: Use the PEBS auto reload mechanism when possible") Link: http://lkml.kernel.org/r/1518474035-21006-2-git-send-email-kan.liang@linux.intel.com Signed-off-by: Ingo Molnar --- arch/x86/events/core.c | 15 +++----- arch/x86/events/intel/ds.c | 92 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 13 deletions(-) diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index 140d33288e78..5a3ccd1715e2 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -1156,16 +1156,13 @@ int x86_perf_event_set_period(struct perf_event *event) per_cpu(pmc_prev_left[idx], smp_processor_id()) = left; - if (!(hwc->flags & PERF_X86_EVENT_AUTO_RELOAD) || - local64_read(&hwc->prev_count) != (u64)-left) { - /* - * The hw event starts counting from this event offset, - * mark it to be able to extra future deltas: - */ - local64_set(&hwc->prev_count, (u64)-left); + /* + * The hw event starts counting from this event offset, + * mark it to be able to extra future deltas: + */ + local64_set(&hwc->prev_count, (u64)-left); - wrmsrl(hwc->event_base, (u64)(-left) & x86_pmu.cntval_mask); - } + wrmsrl(hwc->event_base, (u64)(-left) & x86_pmu.cntval_mask); /* * Due to erratum on certan cpu we need diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c index 18c25ab28557..f39a4df3a7bd 100644 --- a/arch/x86/events/intel/ds.c +++ b/arch/x86/events/intel/ds.c @@ -1306,17 +1306,84 @@ get_next_pebs_record_by_bit(void *base, void *top, int bit) return NULL; } +/* + * Special variant of intel_pmu_save_and_restart() for auto-reload. + */ +static int +intel_pmu_save_and_restart_reload(struct perf_event *event, int count) +{ + struct hw_perf_event *hwc = &event->hw; + int shift = 64 - x86_pmu.cntval_bits; + u64 period = hwc->sample_period; + u64 prev_raw_count, new_raw_count; + s64 new, old; + + WARN_ON(!period); + + /* + * drain_pebs() only happens when the PMU is disabled. + */ + WARN_ON(this_cpu_read(cpu_hw_events.enabled)); + + prev_raw_count = local64_read(&hwc->prev_count); + rdpmcl(hwc->event_base_rdpmc, new_raw_count); + local64_set(&hwc->prev_count, new_raw_count); + + /* + * Since the counter increments a negative counter value and + * overflows on the sign switch, giving the interval: + * + * [-period, 0] + * + * the difference between two consequtive reads is: + * + * A) value2 - value1; + * when no overflows have happened in between, + * + * B) (0 - value1) + (value2 - (-period)); + * when one overflow happened in between, + * + * C) (0 - value1) + (n - 1) * (period) + (value2 - (-period)); + * when @n overflows happened in between. + * + * Here A) is the obvious difference, B) is the extension to the + * discrete interval, where the first term is to the top of the + * interval and the second term is from the bottom of the next + * interval and C) the extension to multiple intervals, where the + * middle term is the whole intervals covered. + * + * An equivalent of C, by reduction, is: + * + * value2 - value1 + n * period + */ + new = ((s64)(new_raw_count << shift) >> shift); + old = ((s64)(prev_raw_count << shift) >> shift); + local64_add(new - old + count * period, &event->count); + + perf_event_update_userpage(event); + + return 0; +} + static void __intel_pmu_pebs_event(struct perf_event *event, struct pt_regs *iregs, void *base, void *top, int bit, int count) { + struct hw_perf_event *hwc = &event->hw; struct perf_sample_data data; struct pt_regs regs; void *at = get_next_pebs_record_by_bit(base, top, bit); - if (!intel_pmu_save_and_restart(event) && - !(event->hw.flags & PERF_X86_EVENT_AUTO_RELOAD)) + if (hwc->flags & PERF_X86_EVENT_AUTO_RELOAD) { + /* + * Now, auto-reload is only enabled in fixed period mode. + * The reload value is always hwc->sample_period. + * May need to change it, if auto-reload is enabled in + * freq mode later. + */ + intel_pmu_save_and_restart_reload(event, count); + } else if (!intel_pmu_save_and_restart(event)) return; while (count > 1) { @@ -1368,8 +1435,11 @@ static void intel_pmu_drain_pebs_core(struct pt_regs *iregs) return; n = top - at; - if (n <= 0) + if (n <= 0) { + if (event->hw.flags & PERF_X86_EVENT_AUTO_RELOAD) + intel_pmu_save_and_restart_reload(event, 0); return; + } __intel_pmu_pebs_event(event, iregs, at, top, 0, n); } @@ -1392,8 +1462,22 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs) ds->pebs_index = ds->pebs_buffer_base; - if (unlikely(base >= top)) + if (unlikely(base >= top)) { + /* + * The drain_pebs() could be called twice in a short period + * for auto-reload event in pmu::read(). There are no + * overflows have happened in between. + * It needs to call intel_pmu_save_and_restart_reload() to + * update the event->count for this case. + */ + for_each_set_bit(bit, (unsigned long *)&cpuc->pebs_enabled, + x86_pmu.max_pebs_events) { + event = cpuc->events[bit]; + if (event->hw.flags & PERF_X86_EVENT_AUTO_RELOAD) + intel_pmu_save_and_restart_reload(event, 0); + } return; + } for (at = base; at < top; at += x86_pmu.pebs_record_size) { struct pebs_record_nhm *p = at;