kcsan: Optimize debugfs stats counters
authorMarco Elver <elver@google.com>
Mon, 10 Aug 2020 08:06:25 +0000 (10:06 +0200)
committerPaul E. McKenney <paulmck@kernel.org>
Mon, 24 Aug 2020 22:10:23 +0000 (15:10 -0700)
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 <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
kernel/kcsan/core.c
kernel/kcsan/debugfs.c
kernel/kcsan/kcsan.h
kernel/kcsan/report.c

index b176400..8a1ff60 100644 (file)
@@ -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);
 }
@@ -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;
        }
 
@@ -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
@@ -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,
@@ -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);
index 6c4914f..3c8093a 100644 (file)
 
 #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",
@@ -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
@@ -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);
index 2948001..8d4bf34 100644 (file)
@@ -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>
 
@@ -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.
@@ -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
index bf1d594..d3bf87e 100644 (file)
@@ -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;
        }