Commit 0c99241c authored by Ingo Molnar's avatar Ingo Molnar

perf/x86/intel/pt: Fix and clean up error handling in pt_event_add()

Dan Carpenter reported that pt_event_add() has buggy
error handling logic: it returns 0 instead of -EBUSY when
it fails to start a newly added event.

Furthermore, the control flow in this function is messy,
with cleanup labels mixed with direct returns.

Fix the bug and clean up the code by converting it to
a straight fast path for the regular non-failing case,
plus a clear sequence of cascading goto labels to do
all cleanup.

NOTE: I materially changed the existing clean up logic in the
pt_event_start() failure case to use the direct
perf_aux_output_end() path, not pt_event_del(), because
perf_aux_output_end() is enough here.
Reported-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Acked-by: default avatarAlexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150416103830.GB7847@gmail.comSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 78d504bc
...@@ -988,39 +988,36 @@ static int pt_event_add(struct perf_event *event, int mode) ...@@ -988,39 +988,36 @@ static int pt_event_add(struct perf_event *event, int mode)
int ret = -EBUSY; int ret = -EBUSY;
if (pt->handle.event) if (pt->handle.event)
goto out; goto fail;
buf = perf_aux_output_begin(&pt->handle, event); buf = perf_aux_output_begin(&pt->handle, event);
if (!buf) { ret = -EINVAL;
ret = -EINVAL; if (!buf)
goto out; goto fail_stop;
}
pt_buffer_reset_offsets(buf, pt->handle.head); pt_buffer_reset_offsets(buf, pt->handle.head);
if (!buf->snapshot) { if (!buf->snapshot) {
ret = pt_buffer_reset_markers(buf, &pt->handle); ret = pt_buffer_reset_markers(buf, &pt->handle);
if (ret) { if (ret)
perf_aux_output_end(&pt->handle, 0, true); goto fail_end_stop;
goto out;
}
} }
if (mode & PERF_EF_START) { if (mode & PERF_EF_START) {
pt_event_start(event, 0); pt_event_start(event, 0);
if (hwc->state == PERF_HES_STOPPED) { ret = -EBUSY;
pt_event_del(event, 0); if (hwc->state == PERF_HES_STOPPED)
ret = -EBUSY; goto fail_end_stop;
}
} else { } else {
hwc->state = PERF_HES_STOPPED; hwc->state = PERF_HES_STOPPED;
} }
ret = 0; return 0;
out:
if (ret)
hwc->state = PERF_HES_STOPPED;
fail_end_stop:
perf_aux_output_end(&pt->handle, 0, true);
fail_stop:
hwc->state = PERF_HES_STOPPED;
fail:
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment