* [PATCH 1/7] x86/perf/amd: Remove need to check "running" bit in NMI handler
2019-04-12 14:22 [GIT PULL 0/7] perf/urgent fixes Arnaldo Carvalho de Melo
@ 2019-04-12 14:22 ` Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 2/7] x86/perf/amd: Fix build failure when CONFIG_HAVE_NMI_WATCHDOG is not set Arnaldo Carvalho de Melo
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-12 14:22 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
linux-perf-users, Lendacky, Thomas, Tom Lendacky, Peter Zijlstra,
stable, Alexander Shishkin, Arnaldo Carvalho de Melo,
Arnaldo Carvalho de Melo, Borislav Petkov, Jiri Olsa,
Linus Torvalds, Stephane Eranian, Vince Weaver
From: "Lendacky, Thomas" <Thomas.Lendacky@amd.com>
Spurious interrupt support was adding to perf in:
commit 63e6be6d98e1 ("perf, x86: Catch spurious interrupts after disabling counters")
The two previous patches (resolving the race condition when disabling a
PMC and NMI latency mitigation) allow for the removal of this older
spurious interrupt support.
Currently in x86_pmu_stop(), the bit for the PMC in the active_mask bitmap
is cleared before disabling the PMC, which sets up a race condition. This
race condition was mitigated by introducing the running bitmap. That race
condition can be eliminated by first disabling the PMC, waiting for PMC
reset on overflow and then clearing the bit for the PMC in the active_mask
bitmap. The NMI handler will not re-enable a disabled counter.
If x86_pmu_stop() is called from the perf NMI handler, the NMI latency
mitigation support will guard against any unhandled NMI messages.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <stable@vger.kernel.org> # 4.14.x-
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: https://lkml.kernel.org/r/Message-ID:
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
arch/x86/events/amd/core.c | 19 ++++++++++++++++++-
arch/x86/events/core.c | 13 +++----------
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 34c191453ce3..5d423653f744 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -491,6 +491,23 @@ static void amd_pmu_disable_all(void)
}
}
+static void amd_pmu_disable_event(struct perf_event *event)
+{
+ x86_pmu_disable_event(event);
+
+ /*
+ * This can be called from NMI context (via x86_pmu_stop). The counter
+ * may have overflowed, but either way, we'll never see it get reset
+ * by the NMI if we're already in the NMI. And the NMI latency support
+ * below will take care of any pending NMI that might have been
+ * generated by the overflow.
+ */
+ if (in_nmi())
+ return;
+
+ amd_pmu_wait_on_overflow(event->hw.idx);
+}
+
/*
* Because of NMI latency, if multiple PMC counters are active or other sources
* of NMIs are received, the perf NMI handler can handle one or more overflowed
@@ -738,7 +755,7 @@ static __initconst const struct x86_pmu amd_pmu = {
.disable_all = amd_pmu_disable_all,
.enable_all = x86_pmu_enable_all,
.enable = x86_pmu_enable_event,
- .disable = x86_pmu_disable_event,
+ .disable = amd_pmu_disable_event,
.hw_config = amd_pmu_hw_config,
.schedule_events = x86_schedule_events,
.eventsel = MSR_K7_EVNTSEL0,
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index e2b1447192a8..81911e11a15d 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -1349,8 +1349,9 @@ void x86_pmu_stop(struct perf_event *event, int flags)
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
struct hw_perf_event *hwc = &event->hw;
- if (__test_and_clear_bit(hwc->idx, cpuc->active_mask)) {
+ if (test_bit(hwc->idx, cpuc->active_mask)) {
x86_pmu.disable(event);
+ __clear_bit(hwc->idx, cpuc->active_mask);
cpuc->events[hwc->idx] = NULL;
WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
hwc->state |= PERF_HES_STOPPED;
@@ -1447,16 +1448,8 @@ int x86_pmu_handle_irq(struct pt_regs *regs)
apic_write(APIC_LVTPC, APIC_DM_NMI);
for (idx = 0; idx < x86_pmu.num_counters; idx++) {
- if (!test_bit(idx, cpuc->active_mask)) {
- /*
- * Though we deactivated the counter some cpus
- * might still deliver spurious interrupts still
- * in flight. Catch them:
- */
- if (__test_and_clear_bit(idx, cpuc->running))
- handled++;
+ if (!test_bit(idx, cpuc->active_mask))
continue;
- }
event = cpuc->events[idx];
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/7] x86/perf/amd: Fix build failure when CONFIG_HAVE_NMI_WATCHDOG is not set
2019-04-12 14:22 [GIT PULL 0/7] perf/urgent fixes Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 1/7] x86/perf/amd: Remove need to check "running" bit in NMI handler Arnaldo Carvalho de Melo
@ 2019-04-12 14:22 ` Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 3/7] perf header: Fix lock/unlock imbalances when processing BPF/BTF info Arnaldo Carvalho de Melo
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-12 14:22 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
linux-perf-users, Lendacky, Thomas, Tom Lendacky,
Alexander Shishkin, Arnaldo Carvalho de Melo, Borislav Petkov,
Jiri Olsa, Linus Torvalds, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: "Lendacky, Thomas" <Thomas.Lendacky@amd.com>
When CONFIG_HAVE_NMI_WATCHDOG is not set, the asm/nmi.h file is not
included. The asm/nmi.h file contains the definitions for NMI_HANDLED
and NMI_DONE which are used in arch/x86/events/amd/core.c. This causes
the build the fail.
Fix the issue by changing the include in arch/x86/events/amd/core.c from
linux/nmi.h to asm/nmi.h.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
arch/x86/events/amd/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 5d423653f744..0ecfac84ba91 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -4,8 +4,8 @@
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/delay.h>
-#include <linux/nmi.h>
#include <asm/apicdef.h>
+#include <asm/nmi.h>
#include "../perf_event.h"
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/7] perf header: Fix lock/unlock imbalances when processing BPF/BTF info
2019-04-12 14:22 [GIT PULL 0/7] perf/urgent fixes Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 1/7] x86/perf/amd: Remove need to check "running" bit in NMI handler Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 2/7] x86/perf/amd: Fix build failure when CONFIG_HAVE_NMI_WATCHDOG is not set Arnaldo Carvalho de Melo
@ 2019-04-12 14:22 ` Arnaldo Carvalho de Melo
2019-04-12 16:14 ` Song Liu
2019-04-12 14:22 ` [PATCH 4/7] perf scripts python: export-to-sqlite.py: Fix use of parent_id in calls_view Arnaldo Carvalho de Melo
` (3 subsequent siblings)
6 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-12 14:22 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
linux-perf-users, Gustavo A. R. Silva, Song Liu,
Alexander Shishkin, Jiri Olsa, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Fix lock/unlock imbalances by refactoring the code a bit and adding
calls to up_write() before return.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Acked-by: Song Liu <songliubraving@fb.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Addresses-Coverity-ID: 1444315 ("Missing unlock")
Addresses-Coverity-ID: 1444316 ("Missing unlock")
Fixes: a70a1123174a ("perf bpf: Save BTF information as headers to perf.data")
Fixes: 606f972b1361 ("perf bpf: Save bpf_prog_info information as headers to perf.data")
Link: http://lkml.kernel.org/r/20190408173355.GA10501@embeddedor
[ Simplified the exit path to have just one up_write() + return ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
| 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index b9e693825873..2d2af2ac2b1e 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2606,6 +2606,7 @@ static int process_bpf_prog_info(struct feat_fd *ff, void *data __maybe_unused)
perf_env__insert_bpf_prog_info(env, info_node);
}
+ up_write(&env->bpf_progs.lock);
return 0;
out:
free(info_linear);
@@ -2623,7 +2624,9 @@ static int process_bpf_prog_info(struct feat_fd *ff __maybe_unused, void *data _
static int process_bpf_btf(struct feat_fd *ff, void *data __maybe_unused)
{
struct perf_env *env = &ff->ph->env;
+ struct btf_node *node = NULL;
u32 count, i;
+ int err = -1;
if (ff->ph->needs_swap) {
pr_warning("interpreting btf from systems with endianity is not yet supported\n");
@@ -2636,31 +2639,32 @@ static int process_bpf_btf(struct feat_fd *ff, void *data __maybe_unused)
down_write(&env->bpf_progs.lock);
for (i = 0; i < count; ++i) {
- struct btf_node *node;
u32 id, data_size;
if (do_read_u32(ff, &id))
- return -1;
+ goto out;
if (do_read_u32(ff, &data_size))
- return -1;
+ goto out;
node = malloc(sizeof(struct btf_node) + data_size);
if (!node)
- return -1;
+ goto out;
node->id = id;
node->data_size = data_size;
- if (__do_read(ff, node->data, data_size)) {
- free(node);
- return -1;
- }
+ if (__do_read(ff, node->data, data_size))
+ goto out;
perf_env__insert_btf(env, node);
+ node = NULL;
}
+ err = 0;
+out:
up_write(&env->bpf_progs.lock);
- return 0;
+ free(node);
+ return err;
}
struct feature_ops {
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/7] perf header: Fix lock/unlock imbalances when processing BPF/BTF info
2019-04-12 14:22 ` [PATCH 3/7] perf header: Fix lock/unlock imbalances when processing BPF/BTF info Arnaldo Carvalho de Melo
@ 2019-04-12 16:14 ` Song Liu
0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2019-04-12 16:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Thomas Gleixner, Jiri Olsa, Namhyung Kim,
Clark Williams, LKML, linux-perf-users@vger.kernel.org,
Gustavo A. R. Silva, Alexander Shishkin, Jiri Olsa,
Peter Zijlstra, Arnaldo Carvalho de Melo
> On Apr 12, 2019, at 7:22 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
>
> Fix lock/unlock imbalances by refactoring the code a bit and adding
> calls to up_write() before return.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Addresses-Coverity-ID: 1444315 ("Missing unlock")
> Addresses-Coverity-ID: 1444316 ("Missing unlock")
> Fixes: a70a1123174a ("perf bpf: Save BTF information as headers to perf.data")
> Fixes: 606f972b1361 ("perf bpf: Save bpf_prog_info information as headers to perf.data")
> Link: http://lkml.kernel.org/r/20190408173355.GA10501@embeddedor
> [ Simplified the exit path to have just one up_write() + return ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/perf/util/header.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index b9e693825873..2d2af2ac2b1e 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2606,6 +2606,7 @@ static int process_bpf_prog_info(struct feat_fd *ff, void *data __maybe_unused)
> perf_env__insert_bpf_prog_info(env, info_node);
> }
>
> + up_write(&env->bpf_progs.lock);
> return 0;
> out:
> free(info_linear);
> @@ -2623,7 +2624,9 @@ static int process_bpf_prog_info(struct feat_fd *ff __maybe_unused, void *data _
> static int process_bpf_btf(struct feat_fd *ff, void *data __maybe_unused)
> {
> struct perf_env *env = &ff->ph->env;
> + struct btf_node *node = NULL;
> u32 count, i;
> + int err = -1;
>
> if (ff->ph->needs_swap) {
> pr_warning("interpreting btf from systems with endianity is not yet supported\n");
> @@ -2636,31 +2639,32 @@ static int process_bpf_btf(struct feat_fd *ff, void *data __maybe_unused)
> down_write(&env->bpf_progs.lock);
>
> for (i = 0; i < count; ++i) {
> - struct btf_node *node;
> u32 id, data_size;
>
> if (do_read_u32(ff, &id))
> - return -1;
> + goto out;
> if (do_read_u32(ff, &data_size))
> - return -1;
> + goto out;
>
> node = malloc(sizeof(struct btf_node) + data_size);
> if (!node)
> - return -1;
> + goto out;
>
> node->id = id;
> node->data_size = data_size;
>
> - if (__do_read(ff, node->data, data_size)) {
> - free(node);
> - return -1;
> - }
> + if (__do_read(ff, node->data, data_size))
> + goto out;
>
> perf_env__insert_btf(env, node);
> + node = NULL;
> }
>
> + err = 0;
> +out:
> up_write(&env->bpf_progs.lock);
> - return 0;
> + free(node);
> + return err;
> }
>
> struct feature_ops {
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/7] perf scripts python: export-to-sqlite.py: Fix use of parent_id in calls_view
2019-04-12 14:22 [GIT PULL 0/7] perf/urgent fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2019-04-12 14:22 ` [PATCH 3/7] perf header: Fix lock/unlock imbalances when processing BPF/BTF info Arnaldo Carvalho de Melo
@ 2019-04-12 14:22 ` Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 5/7] perf stat: Disable DIR_FORMAT feature for 'perf stat record' Arnaldo Carvalho de Melo
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-12 14:22 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
linux-perf-users, Adrian Hunter, Jiri Olsa,
Arnaldo Carvalho de Melo
From: Adrian Hunter <adrian.hunter@intel.com>
Fix following error using calls_view:
Query failed: ambiguous column name: parent_id Unable to execute statement
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Fixes: 8ce9a7251d11 ("perf scripts python: export-to-sqlite.py: Export calls parent_id")
Link: http://lkml.kernel.org/r/20190409062557.26138-1-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/scripts/python/export-to-sqlite.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/scripts/python/export-to-sqlite.py b/tools/perf/scripts/python/export-to-sqlite.py
index 3b71902a5a21..bf271fbc3a88 100644
--- a/tools/perf/scripts/python/export-to-sqlite.py
+++ b/tools/perf/scripts/python/export-to-sqlite.py
@@ -331,7 +331,7 @@ if perf_db_export_calls:
'return_id,'
'CASE WHEN flags=0 THEN \'\' WHEN flags=1 THEN \'no call\' WHEN flags=2 THEN \'no return\' WHEN flags=3 THEN \'no call/return\' WHEN flags=6 THEN \'jump\' ELSE flags END AS flags,'
'parent_call_path_id,'
- 'parent_id'
+ 'calls.parent_id'
' FROM calls INNER JOIN call_paths ON call_paths.id = call_path_id')
do_query(query, 'CREATE VIEW samples_view AS '
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/7] perf stat: Disable DIR_FORMAT feature for 'perf stat record'
2019-04-12 14:22 [GIT PULL 0/7] perf/urgent fixes Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2019-04-12 14:22 ` [PATCH 4/7] perf scripts python: export-to-sqlite.py: Fix use of parent_id in calls_view Arnaldo Carvalho de Melo
@ 2019-04-12 14:22 ` Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 6/7] tools lib traceevent: Fix missing equality check for strcmp Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 7/7] perf evsel: Use hweight64() instead of hweight_long(attr.sample_regs_user) Arnaldo Carvalho de Melo
6 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-12 14:22 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
linux-perf-users, Arnaldo Carvalho de Melo, Alexander Shishkin,
Peter Zijlstra
From: Jiri Olsa <jolsa@kernel.org>
Arnaldo reported assertion in perf stat record:
assertion failed at util/header.c:875
There's no support for this in the 'perf state record' command, disable
the feature for that case.
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 258031c017c3 ("perf header: Add DIR_FORMAT feature to describe directory data")
Link: http://lkml.kernel.org/r/20190409100156.20303-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-stat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 49ee3c2033ec..c3625ec374e0 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1308,6 +1308,7 @@ static void init_features(struct perf_session *session)
for (feat = HEADER_FIRST_FEATURE; feat < HEADER_LAST_FEATURE; feat++)
perf_header__set_feat(&session->header, feat);
+ perf_header__clear_feat(&session->header, HEADER_DIR_FORMAT);
perf_header__clear_feat(&session->header, HEADER_BUILD_ID);
perf_header__clear_feat(&session->header, HEADER_TRACING_DATA);
perf_header__clear_feat(&session->header, HEADER_BRANCH_STACK);
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/7] tools lib traceevent: Fix missing equality check for strcmp
2019-04-12 14:22 [GIT PULL 0/7] perf/urgent fixes Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2019-04-12 14:22 ` [PATCH 5/7] perf stat: Disable DIR_FORMAT feature for 'perf stat record' Arnaldo Carvalho de Melo
@ 2019-04-12 14:22 ` Arnaldo Carvalho de Melo
2019-04-12 14:22 ` [PATCH 7/7] perf evsel: Use hweight64() instead of hweight_long(attr.sample_regs_user) Arnaldo Carvalho de Melo
6 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-12 14:22 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
linux-perf-users, Rikard Falkeborn, Steven Rostedt,
Tzvetomir Stoyanov, Arnaldo Carvalho de Melo
From: Rikard Falkeborn <rikard.falkeborn@gmail.com>
There was a missing comparison with 0 when checking if type is "s64" or
"u64". Therefore, the body of the if-statement was entered if "type" was
"u64" or not "s64", which made the first strcmp() redundant since if
type is "u64", it's not "s64".
If type is "s64", the body of the if-statement is not entered but since
the remainder of the function consists of if-statements which will not
be entered if type is "s64", we will just return "val", which is
correct, albeit at the cost of a few more calls to strcmp(), i.e., it
will behave just as if the if-statement was entered.
If type is neither "s64" or "u64", the body of the if-statement will be
entered incorrectly and "val" returned. This means that any type that is
checked after "s64" and "u64" is handled the same way as "s64" and
"u64", i.e., the limiting of "val" to fit in for example "s8" is never
reached.
This was introduced in the kernel tree when the sources were copied from
trace-cmd in commit f7d82350e597 ("tools/events: Add files to create
libtraceevent.a"), and in the trace-cmd repo in 1cdbae6035cei
("Implement typecasting in parser") when the function was introduced,
i.e., it has always behaved the wrong way.
Detected by cppcheck.
Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Fixes: f7d82350e597 ("tools/events: Add files to create libtraceevent.a")
Link: http://lkml.kernel.org/r/20190409091529.2686-1-rikard.falkeborn@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/lib/traceevent/event-parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 87494c7c619d..981c6ce2da2c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2233,7 +2233,7 @@ eval_type_str(unsigned long long val, const char *type, int pointer)
return val & 0xffffffff;
if (strcmp(type, "u64") == 0 ||
- strcmp(type, "s64"))
+ strcmp(type, "s64") == 0)
return val;
if (strcmp(type, "s8") == 0)
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 7/7] perf evsel: Use hweight64() instead of hweight_long(attr.sample_regs_user)
2019-04-12 14:22 [GIT PULL 0/7] perf/urgent fixes Arnaldo Carvalho de Melo
` (5 preceding siblings ...)
2019-04-12 14:22 ` [PATCH 6/7] tools lib traceevent: Fix missing equality check for strcmp Arnaldo Carvalho de Melo
@ 2019-04-12 14:22 ` Arnaldo Carvalho de Melo
6 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-12 14:22 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
linux-perf-users, Mao Han, Adrian Hunter, Alexander Shishkin,
Jiri Olsa, Peter Zijlstra, Stephane Eranian,
Arnaldo Carvalho de Melo
From: Mao Han <han_mao@c-sky.com>
On 32-bits platform with more than 32 registers, the 64 bits mask is
truncate to the lower 32 bits and the return value of hweight_long will
always smaller than 32. When kernel outputs more than 32 registers, but
the user perf program only counts 32, there will be a data mismatch
result to overflow check fail.
Signed-off-by: Mao Han <han_mao@c-sky.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Fixes: 6a21c0b5c2ab ("perf tools: Add core support for sampling intr machine state regs")
Fixes: d03f2170546d ("perf tools: Expand perf_event__synthesize_sample()")
Fixes: 0f6a30150ca2 ("perf tools: Support user regs and stack in sample parsing")
Link: http://lkml.kernel.org/r/29ad7947dc8fd1ff0abd2093a72cc27a2446be9f.1554883878.git.han_mao@c-sky.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evsel.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 66d066f18b5b..966360844fff 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2368,7 +2368,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
if (data->user_regs.abi) {
u64 mask = evsel->attr.sample_regs_user;
- sz = hweight_long(mask) * sizeof(u64);
+ sz = hweight64(mask) * sizeof(u64);
OVERFLOW_CHECK(array, sz, max_size);
data->user_regs.mask = mask;
data->user_regs.regs = (u64 *)array;
@@ -2424,7 +2424,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
if (data->intr_regs.abi != PERF_SAMPLE_REGS_ABI_NONE) {
u64 mask = evsel->attr.sample_regs_intr;
- sz = hweight_long(mask) * sizeof(u64);
+ sz = hweight64(mask) * sizeof(u64);
OVERFLOW_CHECK(array, sz, max_size);
data->intr_regs.mask = mask;
data->intr_regs.regs = (u64 *)array;
@@ -2552,7 +2552,7 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
if (type & PERF_SAMPLE_REGS_USER) {
if (sample->user_regs.abi) {
result += sizeof(u64);
- sz = hweight_long(sample->user_regs.mask) * sizeof(u64);
+ sz = hweight64(sample->user_regs.mask) * sizeof(u64);
result += sz;
} else {
result += sizeof(u64);
@@ -2580,7 +2580,7 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
if (type & PERF_SAMPLE_REGS_INTR) {
if (sample->intr_regs.abi) {
result += sizeof(u64);
- sz = hweight_long(sample->intr_regs.mask) * sizeof(u64);
+ sz = hweight64(sample->intr_regs.mask) * sizeof(u64);
result += sz;
} else {
result += sizeof(u64);
@@ -2710,7 +2710,7 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type,
if (type & PERF_SAMPLE_REGS_USER) {
if (sample->user_regs.abi) {
*array++ = sample->user_regs.abi;
- sz = hweight_long(sample->user_regs.mask) * sizeof(u64);
+ sz = hweight64(sample->user_regs.mask) * sizeof(u64);
memcpy(array, sample->user_regs.regs, sz);
array = (void *)array + sz;
} else {
@@ -2746,7 +2746,7 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type,
if (type & PERF_SAMPLE_REGS_INTR) {
if (sample->intr_regs.abi) {
*array++ = sample->intr_regs.abi;
- sz = hweight_long(sample->intr_regs.mask) * sizeof(u64);
+ sz = hweight64(sample->intr_regs.mask) * sizeof(u64);
memcpy(array, sample->intr_regs.regs, sz);
array = (void *)array + sz;
} else {
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread