perf: Fix the software events state check
PATCH 1/2 perf: Fix the software events state check
Fix the mistakenly inverted check of events state.
--- kernel/perf_event.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/perf_event.c +1 -1
@@ -5122,7 +5122,7 @@ static int perf_exclude_event(struct perf_event *event, struct pt_regs *regs) {if (event->hw.state &PERF_HES_STOPPED ) -return 0 ; +return 1 ;if (regs) {if (event->attr.exclude_user &&user_mode (regs))
Signature · 1 line
1.7.3.2
PATCH 2/2 perf: Handle stopped state with tracepoints
We toggle the state from start and stop callbacks but actually don't check it when the event triggers. Do it so that these callbacks actually work.
--- kernel/perf_event.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
kernel/perf_event.c +2 -0
@@ -5478,6 +5478,8 @@ static int perf_tp_event_match(struct perf_event *event, struct perf_sample_data *data, struct pt_regs *regs) { +if (event->hw.state &PERF_HES_STOPPED ) +return 0 ;/* * All tracepoints are from kernel-space. */
Signature · 1 line
1.7.3.2
[tip:perf/urgent] perf: Handle stopped state with tracepoints
Commit-ID: a0f7d0f7fc02465bb9758501f611f63381792996 Gitweb: http://git.kernel.org/tip/a0f7d0f7fc02465bb9758501f611f63381792996 Author: Frederic Weisbecker <[email protected]> AuthorDate: Mon, 7 Mar 2011 21:27:09 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 16 Mar 2011 14:04:14 +0100
perf: Handle stopped state with tracepoints
We toggle the state from start and stop callbacks but actually don't check it when the event triggers. Do it so that these callbacks actually work.
LKML-Reference: <[email protected]>
--- kernel/perf_event.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
kernel/perf_event.c +2 -0
@@ -5478,6 +5478,8 @@ static int perf_tp_event_match(struct perf_event *event, struct perf_sample_data *data, struct pt_regs *regs) { +if (event->hw.state &PERF_HES_STOPPED ) +return 0 ;/* * All tracepoints are from kernel-space. */
Quoted 17 lines · Frederic Weisbecker
Fix the mistakenly inverted check of events state.
Signed-off-by Frederic Weisbecker <[email protected]>Cc Ingo Molnar <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Paul Mackerras <[email protected]>Cc Stephane Eranian <[email protected]>--- kernel/perf_event.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)kernel/perf_event.c +0
-0
kernel/perf_event.c static int perf_exclude_event(struct perf_event *event, · line 5125
Quoted 5 lines
kernel/perf_event.c +1
-1@@ -5122,7 +5122,7 @@ static int perf_exclude_event(struct perf_event *event, struct pt_regs *regs) {if (event->hw.state &PERF_HES_STOPPED ) -return 0 ; +return 1 ;
Just wondering, what was/is the practical effect of this bug, and how far back does it go (any need for a -stable tag)?
Thanks,
Ingo
Quoted 15 lines · Ingo Molnar
Quoted 12 lines · Frederic Weisbecker
Fix the mistakenly inverted check of events state.
Signed-off-by Frederic Weisbecker <[email protected]>Cc Ingo Molnar <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Paul Mackerras <[email protected]>Cc Stephane Eranian <[email protected]>--- kernel/perf_event.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 13 lines
kernel/perf_event.c static int perf_exclude_event(struct perf_event *event, · line 5125
Quoted 10 lines
kernel/perf_event.c +1
-1@@ -5122,7 +5122,7 @@ static int perf_exclude_event(struct perf_event *event, struct pt_regs *regs) {if (event->hw.state &PERF_HES_STOPPED ) -return 0 ; +return 1 ;Just wondering, what was/is the practical effect of this bug, and how far back does it go (any need for a -stable tag)?
I wasn't sure about that so I haven't told about the impact in the changelog.
But now that I put a deeper look into this:
It seems that ->stop() / ->start() are called from perf_adjust_period() to update the hardware with the new settings of period_left. The events are stopped to avoid any race with events triggering with a stale period_left in the hardware level when the software one has been updated, I guess.
So it doesn't seem to fix any existing bug because for ->stop() and ->start() are only useful for hardware events right now. But we may call ->stop() and ->start() for further purpose later. In fact that paves the way for the event exclusion patchset I'm about to post.
So it should be .39 material. But a confirmation from Peter would be nice.
Quoted 6 lines · Frederic Weisbecker
It seems that ->stop() / ->start() are called from
perf_adjust_period()to update the hardware with the new settings ofperiod_left. The events are stopped to avoid any race with events triggering with a staleperiod_leftin the hardware level when the software one has been updated, I guess.
Quoted 6 lines
So it doesn't seem to fix any existing bug because for ->stop() and ->start() are only useful for hardware events right now. But we may call ->stop() and ->start() for further purpose later. In fact that paves the way for the event exclusion patchset I'm about to post.
So it should be .39 material. But a confirmation from Peter would be nice.
Yeah, that sounds about right, I've already queued these patches for .39.
Commit-ID: 91b2f482e62ad0d444222253026a5cbca28c4ab9 Gitweb: http://git.kernel.org/tip/91b2f482e62ad0d444222253026a5cbca28c4ab9 Author: Frederic Weisbecker <[email protected]> AuthorDate: Mon, 7 Mar 2011 21:27:08 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 16 Mar 2011 14:04:13 +0100
perf: Fix the software events state check
Fix the mistakenly inverted check of events state.
LKML-Reference: <[email protected]>
--- kernel/perf_event.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/perf_event.c +1 -1
@@ -5122,7 +5122,7 @@ static int perf_exclude_event(struct perf_event *event, struct pt_regs *regs) {if (event->hw.state &PERF_HES_STOPPED ) -return 0 ; +return 1 ;if (regs) {if (event->attr.exclude_user &&user_mode (regs))