Skip to content

Commit

Permalink
kcsan: Optimize debugfs stats counters
Browse files Browse the repository at this point in the history
Remove kcsan_counter_inc/dec() functions, as they perform no other
logic, and are no longer needed.

This avoids several calls in kcsan_setup_watchpoint() and
kcsan_found_watchpoint(), as well as lets the compiler warn us about
potential out-of-bounds accesses as the array's size is known at all
usage sites at compile-time.

Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
  • Loading branch information
melver authored and paulmckrcu committed Aug 24, 2020
1 parent 178a187 commit 2e986b8
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 34 deletions.
22 changes: 11 additions & 11 deletions kernel/kcsan/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -367,13 +367,13 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
* already removed the watchpoint, or another thread consumed
* the watchpoint before this thread.
*/
kcsan_counter_inc(KCSAN_COUNTER_REPORT_RACES);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_REPORT_RACES]);
}

if ((type & KCSAN_ACCESS_ASSERT) != 0)
kcsan_counter_inc(KCSAN_COUNTER_ASSERT_FAILURES);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_ASSERT_FAILURES]);
else
kcsan_counter_inc(KCSAN_COUNTER_DATA_RACES);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_DATA_RACES]);

user_access_restore(flags);
}
Expand Down Expand Up @@ -414,7 +414,7 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
goto out;

if (!check_encodable((unsigned long)ptr, size)) {
kcsan_counter_inc(KCSAN_COUNTER_UNENCODABLE_ACCESSES);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_UNENCODABLE_ACCESSES]);
goto out;
}

Expand All @@ -434,12 +434,12 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
* with which should_watch() returns true should be tweaked so
* that this case happens very rarely.
*/
kcsan_counter_inc(KCSAN_COUNTER_NO_CAPACITY);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_NO_CAPACITY]);
goto out_unlock;
}

kcsan_counter_inc(KCSAN_COUNTER_SETUP_WATCHPOINTS);
kcsan_counter_inc(KCSAN_COUNTER_USED_WATCHPOINTS);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_SETUP_WATCHPOINTS]);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_USED_WATCHPOINTS]);

/*
* Read the current value, to later check and infer a race if the data
Expand Down Expand Up @@ -541,16 +541,16 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
* increment this counter.
*/
if (is_assert && value_change == KCSAN_VALUE_CHANGE_TRUE)
kcsan_counter_inc(KCSAN_COUNTER_ASSERT_FAILURES);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_ASSERT_FAILURES]);

kcsan_report(ptr, size, type, value_change, KCSAN_REPORT_RACE_SIGNAL,
watchpoint - watchpoints);
} else if (value_change == KCSAN_VALUE_CHANGE_TRUE) {
/* Inferring a race, since the value should not have changed. */

kcsan_counter_inc(KCSAN_COUNTER_RACES_UNKNOWN_ORIGIN);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_RACES_UNKNOWN_ORIGIN]);
if (is_assert)
kcsan_counter_inc(KCSAN_COUNTER_ASSERT_FAILURES);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_ASSERT_FAILURES]);

if (IS_ENABLED(CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN) || is_assert)
kcsan_report(ptr, size, type, KCSAN_VALUE_CHANGE_TRUE,
Expand All @@ -563,7 +563,7 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
* reused after this point.
*/
remove_watchpoint(watchpoint);
kcsan_counter_dec(KCSAN_COUNTER_USED_WATCHPOINTS);
atomic_long_dec(&kcsan_counters[KCSAN_COUNTER_USED_WATCHPOINTS]);
out_unlock:
if (!kcsan_interrupt_watcher)
local_irq_restore(irq_flags);
Expand Down
21 changes: 5 additions & 16 deletions kernel/kcsan/debugfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@

#include "kcsan.h"

/*
* Statistics counters.
*/
static atomic_long_t counters[KCSAN_COUNTER_COUNT];
atomic_long_t kcsan_counters[KCSAN_COUNTER_COUNT];
static const char *const counter_names[] = {
[KCSAN_COUNTER_USED_WATCHPOINTS] = "used_watchpoints",
[KCSAN_COUNTER_SETUP_WATCHPOINTS] = "setup_watchpoints",
Expand Down Expand Up @@ -53,16 +50,6 @@ static struct {
};
static DEFINE_SPINLOCK(report_filterlist_lock);

void kcsan_counter_inc(enum kcsan_counter_id id)
{
atomic_long_inc(&counters[id]);
}

void kcsan_counter_dec(enum kcsan_counter_id id)
{
atomic_long_dec(&counters[id]);
}

/*
* The microbenchmark allows benchmarking KCSAN core runtime only. To run
* multiple threads, pipe 'microbench=<iters>' from multiple tasks into the
Expand Down Expand Up @@ -206,8 +193,10 @@ static int show_info(struct seq_file *file, void *v)

/* show stats */
seq_printf(file, "enabled: %i\n", READ_ONCE(kcsan_enabled));
for (i = 0; i < KCSAN_COUNTER_COUNT; ++i)
seq_printf(file, "%s: %ld\n", counter_names[i], atomic_long_read(&counters[i]));
for (i = 0; i < KCSAN_COUNTER_COUNT; ++i) {
seq_printf(file, "%s: %ld\n", counter_names[i],
atomic_long_read(&kcsan_counters[i]));
}

/* show filter functions, and filter type */
spin_lock_irqsave(&report_filterlist_lock, flags);
Expand Down
12 changes: 6 additions & 6 deletions kernel/kcsan/kcsan.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#ifndef _KERNEL_KCSAN_KCSAN_H
#define _KERNEL_KCSAN_KCSAN_H

#include <linux/atomic.h>
#include <linux/kcsan.h>
#include <linux/sched.h>

Expand All @@ -34,6 +35,10 @@ void kcsan_restore_irqtrace(struct task_struct *task);
*/
void kcsan_debugfs_init(void);

/*
* Statistics counters displayed via debugfs; should only be modified in
* slow-paths.
*/
enum kcsan_counter_id {
/*
* Number of watchpoints currently in use.
Expand Down Expand Up @@ -86,12 +91,7 @@ enum kcsan_counter_id {

KCSAN_COUNTER_COUNT, /* number of counters */
};

/*
* Increment/decrement counter with given id; avoid calling these in fast-path.
*/
extern void kcsan_counter_inc(enum kcsan_counter_id id);
extern void kcsan_counter_dec(enum kcsan_counter_id id);
extern atomic_long_t kcsan_counters[KCSAN_COUNTER_COUNT];

/*
* Returns true if data races in the function symbol that maps to func_addr
Expand Down
2 changes: 1 addition & 1 deletion kernel/kcsan/report.c
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ static bool prepare_report_consumer(unsigned long *flags,
* If the actual accesses to not match, this was a false
* positive due to watchpoint encoding.
*/
kcsan_counter_inc(KCSAN_COUNTER_ENCODING_FALSE_POSITIVES);
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_ENCODING_FALSE_POSITIVES]);
goto discard;
}

Expand Down

0 comments on commit 2e986b8

Please sign in to comment.