diff --git a/zircon/kernel/arch/arm64/perf_mon.cpp b/zircon/kernel/arch/arm64/perf_mon.cpp
index 6cfb24bb227940c6538bb697ee85296d03d2cbcd..b52a9f58bee01275971290615f1c408855fb8509 100644
--- a/zircon/kernel/arch/arm64/perf_mon.cpp
+++ b/zircon/kernel/arch/arm64/perf_mon.cpp
@@ -762,17 +762,18 @@ static void arm64_perfmon_stop_task(void* raw_context) TA_NO_THREAD_SAFETY_ANALY
arm64_perfmon_clear_overflow_indicators();
}
-// Stop collecting data.
-// It's ok to call this multiple times.
-// Returns an error if called before ALLOC or after FREE.
-zx_status_t arch_perfmon_stop() {
- Guard<Mutex> guard(PerfmonLock::Get());
-
+void arch_perfmon_stop_locked() TA_REQ(PerfmonLock::Get()) {
if (!perfmon_supported) {
- return ZX_ERR_NOT_SUPPORTED;
+ // Nothing to do.
+ return;
}
if (!perfmon_state) {
- return ZX_ERR_BAD_STATE;
+ // Nothing to do.
+ return;
+ }
+ if (!atomic_load(&perfmon_active)) {
+ // Nothing to do.
+ return;
}
TRACEF("Disabling perfmon\n");
@@ -791,8 +792,12 @@ zx_status_t arch_perfmon_stop() {
// Make sure to do this after we've turned everything off so that we
// don't get another PMI after this.
arm64_perfmon_unmap_buffers_locked(state);
+}
- return ZX_OK;
+// Stop collecting data.
+void arch_perfmon_stop() {
+ Guard<Mutex> guard(PerfmonLock::Get());
+ arch_perfmon_stop_locked();
}
// Worker for arm64_perfmon_fini to be executed on all cpus.
@@ -821,22 +826,22 @@ static void arm64_perfmon_reset_task(void* raw_context) TA_NO_THREAD_SAFETY_ANAL
// Finish data collection, reset h/w back to initial state and undo
// everything arm64_perfmon_init did.
-// Must be called while tracing is stopped.
-// It's ok to call this multiple times.
-zx_status_t arch_perfmon_fini() {
+void arch_perfmon_fini() {
Guard<Mutex> guard(PerfmonLock::Get());
if (!perfmon_supported) {
- return ZX_ERR_NOT_SUPPORTED;
+ // Nothing to do.
+ return;
}
+
if (atomic_load(&perfmon_active)) {
- return ZX_ERR_BAD_STATE;
+ arch_perfmon_stop_locked();
+ DEBUG_ASSERT(!atomic_load(&perfmon_active));
}
mp_sync_exec(MP_IPI_TARGET_ALL, 0, arm64_perfmon_reset_task, nullptr);
perfmon_state.reset();
- return ZX_OK;
}
// Interrupt handling.
diff --git a/zircon/kernel/arch/x86/perf_mon.cpp b/zircon/kernel/arch/x86/perf_mon.cpp
index e30c31f9ff902f49bf638563d0536fd1276fb16c..307bae00d7957932c1e3d26305acdf28588b5c9a 100644
--- a/zircon/kernel/arch/x86/perf_mon.cpp
+++ b/zircon/kernel/arch/x86/perf_mon.cpp
@@ -1673,16 +1673,19 @@ static void x86_perfmon_stop_cpu_task(void* raw_context) TA_NO_THREAD_SAFETY_ANA
x86_perfmon_clear_overflow_indicators();
}
-// Stop collecting data.
-// It's ok to call this multiple times.
-// Returns an error if called before ALLOC or after FREE.
-zx_status_t arch_perfmon_stop() {
- Guard<Mutex> guard(PerfmonLock::Get());
-
- if (!perfmon_supported)
- return ZX_ERR_NOT_SUPPORTED;
- if (!perfmon_state)
- return ZX_ERR_BAD_STATE;
+void arch_perfmon_stop_locked() TA_REQ(PerfmonLock::Get()) {
+ if (!perfmon_supported) {
+ // Nothing to do.
+ return;
+ }
+ if (!perfmon_state) {
+ // Nothing to do.
+ return;
+ }
+ if (!atomic_load(&perfmon_active)) {
+ // Nothing to do.
+ return;
+ }
TRACEF("Disabling perfmon\n");
@@ -1700,8 +1703,12 @@ zx_status_t arch_perfmon_stop() {
// Make sure to do this after we've turned everything off so that we
// don't get another PMI after this.
x86_perfmon_unmap_buffers_locked(state);
+}
- return ZX_OK;
+// Stop collecting data.
+void arch_perfmon_stop() {
+ Guard<Mutex> guard(PerfmonLock::Get());
+ arch_perfmon_stop_locked();
}
// Worker for x86_perfmon_fini to be executed on all cpus.
@@ -1730,21 +1737,22 @@ static void x86_perfmon_reset_task(void* raw_context) TA_NO_THREAD_SAFETY_ANALYS
// Finish data collection, reset h/w back to initial state and undo
// everything x86_perfmon_init did.
-// Must be called while tracing is stopped.
-// It's ok to call this multiple times.
-zx_status_t arch_perfmon_fini() {
+void arch_perfmon_fini() {
Guard<Mutex> guard(PerfmonLock::Get());
- if (!perfmon_supported)
- return ZX_ERR_NOT_SUPPORTED;
- if (atomic_load(&perfmon_active))
- return ZX_ERR_BAD_STATE;
+ if (!perfmon_supported) {
+ // Nothing to do.
+ return;
+ }
+
+ if (atomic_load(&perfmon_active)) {
+ arch_perfmon_stop_locked();
+ DEBUG_ASSERT(!atomic_load(&perfmon_active));
+ }
mp_sync_exec(MP_IPI_TARGET_ALL, 0, x86_perfmon_reset_task, nullptr);
perfmon_state.reset();
-
- return ZX_OK;
}
// Interrupt handling.
diff --git a/zircon/kernel/lib/mtrace/mtrace-pmu.cpp b/zircon/kernel/lib/mtrace/mtrace-pmu.cpp
index e0624ce6c478fb4d17792c4021efdcd30e0520fd..490c72864f57f16e311b8b480e90bfae7ecb72f1 100644
--- a/zircon/kernel/lib/mtrace/mtrace-pmu.cpp
+++ b/zircon/kernel/lib/mtrace/mtrace-pmu.cpp
@@ -95,12 +95,14 @@ zx_status_t mtrace_perfmon_control(uint32_t action, uint32_t options,
case MTRACE_PERFMON_STOP:
if (options != 0 || size != 0)
return ZX_ERR_INVALID_ARGS;
- return arch_perfmon_stop();
+ arch_perfmon_stop();
+ return ZX_OK;
case MTRACE_PERFMON_FINI:
if (options != 0 || size != 0)
return ZX_ERR_INVALID_ARGS;
- return arch_perfmon_fini();
+ arch_perfmon_fini();
+ return ZX_OK;
default:
return ZX_ERR_INVALID_ARGS;
diff --git a/zircon/kernel/lib/perfmon/include/lib/perfmon.h b/zircon/kernel/lib/perfmon/include/lib/perfmon.h
index b627026816ed4b637db59eed42a421eef48a9d64..605cadb1cd0dfcd19ebc2462c07fe7e5bb8b8366 100644
--- a/zircon/kernel/lib/perfmon/include/lib/perfmon.h
+++ b/zircon/kernel/lib/perfmon/include/lib/perfmon.h
@@ -83,11 +83,13 @@ zx_status_t arch_perfmon_start();
// Perform MTRACE_PERFMON_STOP: Stop data collection, including collecting
// the final values of the counters and unmapping the VMOs.
-zx_status_t arch_perfmon_stop();
+// It's ok to call this multiple times.
+void arch_perfmon_stop();
// Perform MTRACE_PERFMON_FINI: Terminate data collection, reset all PMU
-// registers.
-zx_status_t arch_perfmon_fini();
+// registers. Data collection is stopped first if necessary.
+// It's ok to call this multiple times.
+void arch_perfmon_fini();
// This section contains helper routines to write perfmon records.
diff --git a/zircon/system/dev/misc/cpu-trace/perf-mon.cpp b/zircon/system/dev/misc/cpu-trace/perf-mon.cpp
index 7a4124295aa6dcb2494bdf38fb9f051eb4bbbb05..2d5218701aebd749820aa72a9a20b4f23a457b33 100644
--- a/zircon/system/dev/misc/cpu-trace/perf-mon.cpp
+++ b/zircon/system/dev/misc/cpu-trace/perf-mon.cpp
@@ -516,36 +516,32 @@ zx_status_t PerfmonDevice::PmuStart() {
fail:
{
- zx_status_t status2 =
+ [[maybe_unused]] zx_status_t status2 =
zx_mtrace_control(resource, MTRACE_KIND_PERFMON,
MTRACE_PERFMON_FINI, 0, nullptr, 0);
- if (status2 != ZX_OK) {
- zxlogf(TRACE, "%s: MTRACE_PERFMON_FINI failed: %d\n", __func__, status2);
- }
assert(status2 == ZX_OK);
return status;
}
}
-zx_status_t PerfmonDevice::PmuStop() {
+void PerfmonDevice::PmuStop() {
zxlogf(TRACE, "%s called\n", __func__);
PmuPerTraceState* per_trace = per_trace_state_.get();
if (!per_trace) {
- return ZX_ERR_BAD_STATE;
+ return;
}
// Please do not use get_root_resource() in new code. See ZX-1467.
zx_handle_t resource = get_root_resource();
- zx_status_t status =
+ [[maybe_unused]] zx_status_t status =
zx_mtrace_control(resource, MTRACE_KIND_PERFMON,
MTRACE_PERFMON_STOP, 0, nullptr, 0);
- if (status == ZX_OK) {
- active_ = false;
- status = zx_mtrace_control(resource, MTRACE_KIND_PERFMON,
- MTRACE_PERFMON_FINI, 0, nullptr, 0);
- }
- return status;
+ assert(status == ZX_OK);
+ active_ = false;
+ status = zx_mtrace_control(resource, MTRACE_KIND_PERFMON,
+ MTRACE_PERFMON_FINI, 0, nullptr, 0);
+ assert(status == ZX_OK);
}
// Dispatch the various kinds of requests.
@@ -603,7 +599,8 @@ zx_status_t PerfmonDevice::IoctlWorker(uint32_t op,
if (cmdlen != 0 || replymax != 0) {
return ZX_ERR_INVALID_ARGS;
}
- return PmuStop();
+ PmuStop();
+ return ZX_OK;
default:
return ZX_ERR_INVALID_ARGS;
diff --git a/zircon/system/dev/misc/cpu-trace/perf-mon.h b/zircon/system/dev/misc/cpu-trace/perf-mon.h
index e761c3dfcd868bc280222aa53ca709d6e15b68ce..1c7d8c56375235a1b26dac4ea02b9eb6bc146754 100644
--- a/zircon/system/dev/misc/cpu-trace/perf-mon.h
+++ b/zircon/system/dev/misc/cpu-trace/perf-mon.h
@@ -143,7 +143,7 @@ class PerfmonDevice : public DeviceType {
zx_status_t PmuStageConfig(const void* cmd, size_t cmdlen);
zx_status_t PmuGetConfig(void* reply, size_t replymax, size_t* out_actual);
zx_status_t PmuStart();
- zx_status_t PmuStop();
+ void PmuStop();
zx_status_t IoctlWorker(uint32_t op, const void* cmd, size_t cmdlen,
void* reply, size_t replymax, size_t* out_actual);