[counters] remove "max" functionality
It cannot be properly implemented lock-free per core. This
can be provided in a lossy way via the usermode kcounter
app.
ZX-3337 #comment progress
Test: see bug above for manual test.
Change-Id: I28f035e3aad92791d91b8acf95624ad77602b820
diff --git a/kernel/lib/counters/include/lib/counters.h b/kernel/lib/counters/include/lib/counters.h
index c732ea2..bdff755 100644
--- a/kernel/lib/counters/include/lib/counters.h
+++ b/kernel/lib/counters/include/lib/counters.h
@@ -25,17 +25,12 @@
// Kernel counters public API:
// 1- define a new counter.
// KCOUNTER(counter_name, "<counter name>");
-// KCOUNTER_MAX(counter_name, "<counter name>");
//
// 2- counters start at zero, increment the counter:
// kcounter_add(counter_name, 1);
-// or
-// kcounter_max(counter_name, value);
//
// By default with KCOUNTER, the `kcounter` presentation will calculate a
-// sum() across cores rather than summing. KCOUNTER_MAX() calculates the max()
-// of the counters across cores.
-//
+// sum() across cores rather than summing.
//
// Naming the counters
// The naming convention is "kernel.subsystem.thing_or_action"
@@ -96,15 +91,30 @@
static int64_t arena_page_end_[] __asm__("kcounters_arena_page_end");
};
-class CounterBase {
+class Counter {
public:
- explicit constexpr CounterBase(const counters::Descriptor* desc) :
+ explicit constexpr Counter(const counters::Descriptor* desc) :
desc_(desc) { }
int64_t Value() const {
return *Slot();
}
+ void Add(int64_t delta) const {
+#if defined(__aarch64__)
+ // Use a relaxed atomic load/store for arm64 to avoid a potentially
+ // nasty race between the regular load/store operations for a +1.
+ // Relaxed atomic load/stores are about as efficient as a regular
+ // load/store.
+ atomic_add_64_relaxed(Slot(), delta);
+#else
+ // x86 can do the add in a single non atomic instruction, so the data
+ // loss of a preemption in the middle of this sequence is fairly
+ // minimal.
+ *Slot() += delta;
+#endif
+ }
+
protected:
int64_t* Slot() const {
return &get_local_percpu()->counters[Index()];
@@ -120,38 +130,6 @@
const counters::Descriptor* desc_;
};
-struct CounterSum : public CounterBase {
- explicit constexpr CounterSum(const counters::Descriptor* desc) :
- CounterBase(desc) { }
- void Add(int64_t delta) const {
-#if defined(__aarch64__)
- // Use a relaxed atomic load/store for arm64 to avoid a potentially
- // nasty race between the regular load/store operations for a +1.
- // Relaxed atomic load/stores are about as efficient as a regular
- // load/store.
- atomic_add_64_relaxed(Slot(), delta);
-#else
- // x86 can do the add in a single non atomic instruction, so the data
- // loss of a preemption in the middle of this sequence is fairly
- // minimal.
- *Slot() += delta;
-#endif
- }
-};
-
-struct CounterMax : public CounterBase {
- explicit constexpr CounterMax(const counters::Descriptor* desc) :
- CounterBase(desc) { }
- void Update(int64_t value) const {
- // TODO(travisg|scottmg): Revisit, consider more efficient arm-specific
- // instruction sequence here.
- int64_t prev_value = atomic_load_64_relaxed(Slot());
- while (prev_value < value &&
- !atomic_cmpxchg_64_relaxed(Slot(), &prev_value, value))
- ;
- }
-};
-
// Define the descriptor and reserve the arena space for the counters.
// Because of -fdata-sections, each kcounter_arena_* array will be
// placed in a .bss.kcounter.* section; kernel.ld recognizes those names
@@ -164,21 +142,11 @@
namespace { \
__USED int64_t kcounter_arena_##var[SMP_MAX_CPUS] __asm__("kcounter." name); \
alignas(counters::Descriptor) __USED __SECTION("kcountdesc." name) const counters::Descriptor kcounter_desc_##var{name, counters::Type::k##type}; \
- constexpr Counter##type var(&kcounter_desc_##var); \
+ constexpr Counter var(&kcounter_desc_##var); \
} // anonymous namespace
#define KCOUNTER(var, name) KCOUNTER_DECLARE(var, name, Sum)
-#define KCOUNTER_MAX(var, name) KCOUNTER_DECLARE(var, name, Max)
-static inline void kcounter_add(const CounterSum& counter, int64_t delta) {
+static inline void kcounter_add(const Counter& counter, int64_t delta) {
counter.Add(delta);
}
-
-static inline void kcounter_max(const CounterMax& counter, int64_t value) {
- counter.Update(value);
-}
-
-static inline void kcounter_max_counter(const CounterMax& counter,
- const CounterBase& other) {
- counter.Update(other.Value());
-}
diff --git a/kernel/lib/heap/cmpctmalloc/cmpctmalloc.cpp b/kernel/lib/heap/cmpctmalloc/cmpctmalloc.cpp
index c798eb7..eb056bb 100644
--- a/kernel/lib/heap/cmpctmalloc/cmpctmalloc.cpp
+++ b/kernel/lib/heap/cmpctmalloc/cmpctmalloc.cpp
@@ -116,8 +116,6 @@
#define LOCAL_TRACE 0
-KCOUNTER_MAX(max_allocation, "kernel.heap.max_allocation");
-
// Use HEAP_ENABLE_TESTS to enable internal testing. The tests are not useful
// when the target system is up. By that time we have done hundreds of allocations
// already.
@@ -904,9 +902,6 @@
if (size == 0u) {
return NULL;
}
-
- kcounter_max(max_allocation, size);
-
// Large allocations are no longer allowed. See ZX-1318 for details.
if (size > (HEAP_LARGE_ALLOC_BYTES - sizeof(header_t))) {
return NULL;
diff --git a/kernel/object/handle.cpp b/kernel/object/handle.cpp
index ed5f165..a6c1cff 100644
--- a/kernel/object/handle.cpp
+++ b/kernel/object/handle.cpp
@@ -24,7 +24,6 @@
KCOUNTER(handle_count_made, "kernel.handles.made");
KCOUNTER(handle_count_duped, "kernel.handles.duped");
KCOUNTER(handle_count_live, "kernel.handles.live");
-KCOUNTER_MAX(handle_count_max_live, "kernel.handles.max_live");
// Masks for building a Handle's base_value, which ProcessDispatcher
// uses to create zx_handle_t values.
@@ -120,7 +119,6 @@
return nullptr;
kcounter_add(handle_count_made, 1);
kcounter_add(handle_count_live, 1);
- kcounter_max_counter(handle_count_max_live, handle_count_live);
return HandleOwner(new (addr) Handle(ktl::move(dispatcher),
rights, base_value));
}
@@ -141,7 +139,6 @@
return nullptr;
kcounter_add(handle_count_duped, 1);
kcounter_add(handle_count_live, 1);
- kcounter_max_counter(handle_count_max_live, handle_count_live);
return HandleOwner(new (addr) Handle(source, rights, base_value));
}
diff --git a/system/utest/kcounter/kcounter-test.cpp b/system/utest/kcounter/kcounter-test.cpp
index e8a4c0b..4e8e952 100644
--- a/system/utest/kcounter/kcounter-test.cpp
+++ b/system/utest/kcounter/kcounter-test.cpp
@@ -84,16 +84,15 @@
const counters::Descriptor& b) {
return strcmp(a.name, b.name) < 0;
});
- return (result.first == desc->end() ? nullptr :
- &desc->descriptor_table[result.first - desc->begin()]);
+ return (result.first == result.second) ?
+ nullptr : &desc->descriptor_table[result.first - desc->begin()];
};
constexpr counters::Descriptor kExpected[] = {
+ {"kernel.counters.magic", counters::Type::kSum},
{"kernel.handles.duped", counters::Type::kSum},
{"kernel.handles.live", counters::Type::kSum},
{"kernel.handles.made", counters::Type::kSum},
- {"kernel.handles.max_live", counters::Type::kMax},
- {"kernel.counters.magic", counters::Type::kSum},
};
for (const auto& ref : kExpected) {
auto found = find(ref);
@@ -111,7 +110,7 @@
case counters::Type::kSum:
value += cpu_value;
break;
- case counters::Type::kMax:
+ case counters::Type::kMax: // Not used, see ZX-3337.
value = std::max(value, cpu_value);
break;
}
