diff --git a/kernel/include/lib/counters.h b/kernel/include/lib/counters.h
index c3b16e48eda530e3058ed9eb4881db94b1cfac29..ec63458ac146fc8dbf477058742eabed8e884d8d 100644
--- a/kernel/include/lib/counters.h
+++ b/kernel/include/lib/counters.h
@@ -37,11 +37,10 @@ __BEGIN_CDECLS
// "kernel.exceptions.fpu"
// "kernel.handles.new"
//
-// Reading the counters in code
-// Don't. The counters are maintained in a per-cpu arena and
-// atomic operations are never used to set their value so
-// they are both imprecise and reflect only the operations
-// on a particular core.
+// Reading the counters in code
+// Don't. The counters are maintained in a per-cpu arena and atomic
+// operations are never used to set their value so they are both
+// imprecise and reflect only the operations on a particular core.
struct k_counter_desc {
const char* name;
diff --git a/kernel/include/lib/counters_private.h b/kernel/include/lib/counters_private.h
new file mode 100644
index 0000000000000000000000000000000000000000..21d5ef29e73fc6db921911e79e2b0de23d4e6b05
--- /dev/null
+++ b/kernel/include/lib/counters_private.h
@@ -0,0 +1,22 @@
+// Copyright 2018 The Fuchsia Authors
+//
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file or at
+// https://opensource.org/licenses/MIT
+
+// These are helper routines used to implement `k counters`, exposed
+// here only for testing purposes. See the implementation for details.
+
+#pragma once
+
+#include <stddef.h>
+#include <stdint.h>
+#include <zircon/compiler.h>
+
+__BEGIN_CDECLS
+
+void counters_clean_up_values(const uint64_t* values_in, uint64_t* values_out, size_t* count_out);
+uint64_t counters_get_percentile(const uint64_t* values, size_t count, uint64_t percentage_dot8);
+bool counters_has_outlier(const uint64_t* values_in);
+
+__END_CDECLS
diff --git a/kernel/lib/counters/counters.cpp b/kernel/lib/counters/counters.cpp
index 74398c5ce410d8ecd1122419e31b7a642a65d55d..0713599f3c881fd3789f7fa943e3bca428fc3e60 100644
--- a/kernel/lib/counters/counters.cpp
+++ b/kernel/lib/counters/counters.cpp
@@ -6,6 +6,7 @@
#include <lib/counters.h>
+#include <stdlib.h>
#include <string.h>
#include <arch/ops.h>
@@ -23,6 +24,7 @@
#include <lk/init.h>
#include <lib/console.h>
+#include <lib/counters_private.h>
// The arena is allocated in kernel.ld linker script.
extern int64_t kcounters_arena[];
@@ -77,7 +79,74 @@ static void counters_init(unsigned level) {
}
}
-static void dump_counter(const k_counter_desc* desc) {
+// Collapse values to only non-zero ones and sort.
+void counters_clean_up_values(const uint64_t* values_in, uint64_t* values_out, size_t* count_out) {
+ assert(values_in != values_out);
+
+ *count_out = 0;
+ for (size_t i = 0; i < SMP_MAX_CPUS; ++i) {
+ if (values_in[i] > 0) {
+ values_out[(*count_out)++] = values_in[i];
+ }
+ }
+
+ qsort(values_out, *count_out, sizeof(uint64_t), [](const void* a, const void* b) {
+ return (*((uint64_t*)a) > *((uint64_t*)b)) - (*((uint64_t*)a) < *((uint64_t*)b));
+ });
+}
+
+static constexpr uint64_t DOT8_SHIFT = 8;
+
+// This calculation tries to match what sheets.google.com uses for QUARTILE()
+// and PERCENTAGE(), however this uses 56.8 rather than floating point.
+// https://en.wikipedia.org/wiki/Percentile#The_linear_interpolation_between_closest_ranks_method
+// This is just a linear interpolation between the items bracketing that
+// percentage. The input value array must be sorted on entry to this function.
+uint64_t counters_get_percentile(const uint64_t* values, size_t count, uint64_t percentage_dot8) {
+ assert(count >= 2);
+
+ uint64_t target_dot8 = (count - 1) * percentage_dot8;
+ uint64_t low_index = target_dot8 >> DOT8_SHIFT;
+ uint64_t high_index = low_index + 1;
+ uint64_t fraction_dot8 = target_dot8 & 0xff;
+
+ uint64_t delta = values[high_index] - values[low_index];
+ return ((values[low_index] << DOT8_SHIFT) + fraction_dot8 * delta);
+}
+
+bool counters_has_outlier(const uint64_t* values_in) {
+ uint64_t values[SMP_MAX_CPUS];
+ size_t count;
+ counters_clean_up_values(values_in, values, &count);
+ if (count < 2)
+ return false;
+
+ // If there's a value that's an outlier per
+ // https://en.wikipedia.org/wiki/Outlier#Tukey's_fences, then we deem it
+ // worth outputting the per-core values. This is not perfect, but it is
+ // somewhat tricky to determine outliers for small data sets. We typically
+ // have something like 4 cores here, and e.g. calculating standard deviation
+ // is not useful for so few values.
+ const uint64_t q1_dot8 = counters_get_percentile(values, count, /*0.25*/ 64);
+ const uint64_t q3_dot8 = counters_get_percentile(values, count, /*0.75*/ 192);
+ const uint64_t k_dot8 = /*1.5*/ 384;
+ const uint64_t q_delta_dot8 = q3_dot8 - q1_dot8;
+ const int64_t low_dot8 =
+ q1_dot8 - static_cast<int64_t>(((k_dot8 * q_delta_dot8) >> DOT8_SHIFT));
+ const int64_t high_dot8 =
+ q3_dot8 + static_cast<int64_t>(((k_dot8 * q_delta_dot8) >> DOT8_SHIFT));
+
+ for (size_t i = 0; i < count; ++i) {
+ if ((static_cast<int64_t>(values[i]) << DOT8_SHIFT) < low_dot8 ||
+ (static_cast<int64_t>(values[i]) << DOT8_SHIFT) > high_dot8) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static void dump_counter(const k_counter_desc* desc, bool verbose) {
size_t counter_index = kcounter_index(desc);
uint64_t sum = 0;
@@ -93,23 +162,28 @@ static void dump_counter(const k_counter_desc* desc) {
if (sum == 0u)
return;
- // Print the per-core counts when the sum is not zero.
- printf(" ");
- for (size_t ix = 0; ix != SMP_MAX_CPUS; ++ix) {
- if (values[ix] > 0)
- printf("[%zu:%lu]", ix, values[ix]);
+ // Print the per-core counts if verbose (-v) is set and it's not zero, or if
+ // a value for one of the cores indicates it's an outlier.
+ if (verbose || counters_has_outlier(values)) {
+ printf(" ");
+ for (size_t ix = 0; ix != SMP_MAX_CPUS; ++ix) {
+ if (values[ix] > 0) {
+ printf("[%zu:%lu]", ix, values[ix]);
+ }
+ }
+ printf("\n");
}
- printf("\n");
}
-static void dump_all_counters() {
+static void dump_all_counters(bool verbose) {
printf("%zu counters available:\n", get_num_counters());
for (auto it = kcountdesc_begin; it != kcountdesc_end; ++it) {
- dump_counter(it);
+ dump_counter(it, verbose);
}
}
static int watcher_thread_fn(void* arg) {
+ bool verbose = static_cast<bool>(reinterpret_cast<uintptr_t>(arg));
while (true) {
{
fbl::AutoLock lock(&watcher_lock);
@@ -120,7 +194,7 @@ static int watcher_thread_fn(void* arg) {
watched_counter_t* wc;
list_for_every_entry (&watcher_list, wc, watched_counter_t, node) {
- dump_counter(wc->desc);
+ dump_counter(wc->desc, verbose);
}
}
@@ -129,9 +203,18 @@ static int watcher_thread_fn(void* arg) {
}
static int view_counter(int argc, const cmd_args* argv) {
+ bool verbose = false;
+ if (argc == 3) {
+ if (strcmp(argv[1].str, "-v") == 0) {
+ verbose = true;
+ argc--;
+ argv++;
+ }
+ }
+
if (argc == 2) {
- if (strcmp(argv[1].str, "--all") == 0) {
- dump_all_counters();
+ if (strcmp(argv[1].str, "all") == 0) {
+ dump_all_counters(verbose);
} else {
int num_results = 0;
auto name = argv[1].str;
@@ -139,21 +222,21 @@ static int view_counter(int argc, const cmd_args* argv) {
while (desc != kcountdesc_end) {
if (!prefix_match(name, desc->name))
break;
- dump_counter(desc);
+ dump_counter(desc, verbose);
++num_results;
++desc;
}
if (num_results == 0) {
- printf("counter '%s' not found, try --all\n", name);
+ printf("counter '%s' not found, try all\n", name);
} else {
printf("%d counters found\n", num_results);
}
}
} else {
printf(
- "counters view <counter-name>\n"
- "counters view <counter-prefix>\n"
- "counters view --all\n"
+ "counters view [-v] <counter-name>\n"
+ "counters view [-v] <counter-prefix>\n"
+ "counters view [-v] all\n"
);
return 1;
}
@@ -162,8 +245,17 @@ static int view_counter(int argc, const cmd_args* argv) {
}
static int watch_counter(int argc, const cmd_args* argv) {
+ bool verbose = false;
+ if (argc == 3) {
+ if (strcmp(argv[1].str, "-v") == 0) {
+ verbose = true;
+ argc--;
+ argv++;
+ }
+ }
+
if (argc == 2) {
- if (strcmp(argv[1].str, "--stop") == 0) {
+ if (strcmp(argv[1].str, "stop") == 0) {
fbl::AutoLock lock(&watcher_lock);
watched_counter_t* wc;
while ((wc = list_remove_head_type(
@@ -198,7 +290,8 @@ static int watch_counter(int argc, const cmd_args* argv) {
list_add_head(&watcher_list, &wc->node);
if (watcher_thread == nullptr) {
watcher_thread = thread_create(
- "counter-watcher", watcher_thread_fn, nullptr, LOW_PRIORITY);
+ "counter-watcher", watcher_thread_fn,
+ reinterpret_cast<void*>(static_cast<uintptr_t>(verbose)), LOW_PRIORITY);
if (watcher_thread == nullptr) {
printf("no memory for watcher thread\n");
return 1;
@@ -209,8 +302,8 @@ static int watch_counter(int argc, const cmd_args* argv) {
} else {
printf(
- "counters watch <counter-id>\n"
- "counters watch --stop\n"
+ "counters watch [-v] <counter-id>\n"
+ "counters watch stop\n"
);
}
@@ -229,8 +322,8 @@ static int cmd_counters(int argc, const cmd_args* argv, uint32_t flags) {
printf(
"inspect system counters:\n"
- " counters view <name>\n"
- " counters watch <id>\n"
+ " counters view [-v] <name>\n"
+ " counters watch [-v] <id>\n"
);
return 0;
}
diff --git a/kernel/lib/counters/counters_tests.cpp b/kernel/lib/counters/counters_tests.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..3770ef0453cad0e7a08c3c37f828f41b870d52a3
--- /dev/null
+++ b/kernel/lib/counters/counters_tests.cpp
@@ -0,0 +1,81 @@
+// Copyright 2018 The Fuchsia Authors
+//
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file or at
+// https://opensource.org/licenses/MIT
+
+#include <lib/counters_private.h>
+
+#include <lib/unittest/unittest.h>
+
+static bool value_cleanup() {
+ BEGIN_TEST;
+
+ uint64_t outputs[SMP_MAX_CPUS];
+ size_t out_count;
+
+ // Sorted.
+ uint64_t inputs0[SMP_MAX_CPUS] = { 13, 4, 8, 9 };
+ counters_clean_up_values(inputs0, outputs, &out_count);
+ ASSERT_EQ(out_count, 4u, "");
+ EXPECT_EQ(outputs[0], 4u, "");
+ EXPECT_EQ(outputs[1], 8u, "");
+ EXPECT_EQ(outputs[2], 9u, "");
+ EXPECT_EQ(outputs[3], 13u, "");
+
+ // Collapsed to remove zeros.
+ uint64_t inputs1[SMP_MAX_CPUS] = { 13, 0, 0, 9 };
+ counters_clean_up_values(inputs1, outputs, &out_count);
+ ASSERT_EQ(out_count, 2u, "");
+ EXPECT_EQ(outputs[0], 9u, "");
+ EXPECT_EQ(outputs[1], 13u, "");
+
+ END_TEST;
+}
+
+// Data to compare vs. results in
+// https://docs.google.com/spreadsheets/d/1D58chwOpO-3_c41NMGJkpmFuOpGSYH-W50bD6MdOAjo/edit?usp=sharing
+static uint64_t test_counters_inputs0[SMP_MAX_CPUS] = {5105, 4602, 4031, 4866};
+static uint64_t test_counters_inputs1[SMP_MAX_CPUS] = {3524, 3461, 3567, 2866};
+
+static bool percentile_determination() {
+ BEGIN_TEST;
+
+ uint64_t cleaned[SMP_MAX_CPUS];
+ size_t out_count;
+
+ counters_clean_up_values(test_counters_inputs0, cleaned, &out_count);
+ EXPECT_EQ(counters_get_percentile(cleaned, out_count, /*0.25*/ 64),
+ /* 4459.25 */ (4459u << 8) + 64u, "");
+ EXPECT_EQ(counters_get_percentile(cleaned, out_count, /*0.75*/ 192),
+ /* 4925.75 */ (4925u << 8) + 192u, "");
+
+ counters_clean_up_values(test_counters_inputs1, cleaned, &out_count);
+ EXPECT_EQ(counters_get_percentile(cleaned, out_count, /*0.25*/ 64),
+ /* 3312.25 */ (3312u << 8) + 64u, "");
+ EXPECT_EQ(counters_get_percentile(cleaned, out_count, /*0.75*/ 192),
+ /* 3534.75 */ (3534u << 8) + 192u, "");
+
+ END_TEST;
+}
+
+static bool outlier_check() {
+ BEGIN_TEST;
+
+ uint64_t no_values[SMP_MAX_CPUS] = {0};
+ EXPECT_FALSE(counters_has_outlier(no_values), "0 values shouldn't have outlier");
+
+ uint64_t one_value[SMP_MAX_CPUS] = {789};
+ EXPECT_FALSE(counters_has_outlier(one_value), "1 value shouldn't have outlier");
+
+ EXPECT_FALSE(counters_has_outlier(test_counters_inputs0), "");
+ EXPECT_TRUE(counters_has_outlier(test_counters_inputs1), "");
+
+ END_TEST;
+}
+
+UNITTEST_START_TESTCASE(counters_tests)
+UNITTEST("value cleanup", value_cleanup)
+UNITTEST("percentile determination", percentile_determination)
+UNITTEST("outlier check", outlier_check)
+UNITTEST_END_TESTCASE(counters_tests, "counters_tests", "Counters tests");
diff --git a/kernel/lib/counters/rules.mk b/kernel/lib/counters/rules.mk
index b1a8920aefc0ae44517f0ddc1e65c690d903cc82..96cec0026ec4129f063bf8fc32a27afc4f330cf4 100644
--- a/kernel/lib/counters/rules.mk
+++ b/kernel/lib/counters/rules.mk
@@ -9,9 +9,11 @@ LOCAL_DIR := $(GET_LOCAL_DIR)
MODULE := $(LOCAL_DIR)
MODULE_SRCS += \
- $(LOCAL_DIR)/counters.cpp
+ $(LOCAL_DIR)/counters.cpp \
+ $(LOCAL_DIR)/counters_tests.cpp
MODULE_DEPS += \
- kernel/lib/console
+ kernel/lib/console \
+ kernel/lib/unittest
include make/module.mk