From: Marco Elver <elver@google.com>
To: elver@google.com
Cc: paulmck@kernel.org, andreyknvl@google.com, glider@google.com,
dvyukov@google.com, kasan-dev@googlegroups.com,
linux-kernel@vger.kernel.org
Subject: [PATCH -rcu 1/2] kcsan: Show full access type in report
Date: Thu, 9 Jan 2020 16:23:21 +0100 [thread overview]
Message-ID: <20200109152322.104466-2-elver@google.com> (raw)
In-Reply-To: <20200109152322.104466-1-elver@google.com>
This change adds support for showing the complete access type in the
report. Currently the following access types can be shown:
"read", "read (marked)", "write", "write (marked)".
Suggested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Marco Elver <elver@google.com>
---
kernel/kcsan/core.c | 15 ++++++++-------
kernel/kcsan/kcsan.h | 2 +-
kernel/kcsan/report.c | 43 ++++++++++++++++++++++++++++---------------
3 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
index 4d4ab5c5dc53..87bf857c8893 100644
--- a/kernel/kcsan/core.c
+++ b/kernel/kcsan/core.c
@@ -255,7 +255,7 @@ static inline unsigned int get_delay(void)
static noinline void kcsan_found_watchpoint(const volatile void *ptr,
size_t size,
- bool is_write,
+ int type,
atomic_long_t *watchpoint,
long encoded_watchpoint)
{
@@ -276,7 +276,7 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
flags = user_access_save();
if (consumed) {
- kcsan_report(ptr, size, is_write, true, raw_smp_processor_id(),
+ kcsan_report(ptr, size, type, true, raw_smp_processor_id(),
KCSAN_REPORT_CONSUMED_WATCHPOINT);
} else {
/*
@@ -292,8 +292,9 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
}
static noinline void
-kcsan_setup_watchpoint(const volatile void *ptr, size_t size, bool is_write)
+kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
{
+ const bool is_write = (type & KCSAN_ACCESS_WRITE) != 0;
atomic_long_t *watchpoint;
union {
u8 _1;
@@ -415,13 +416,13 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, bool is_write)
* No need to increment 'data_races' counter, as the racing
* thread already did.
*/
- kcsan_report(ptr, size, is_write, size > 8 || value_change,
+ kcsan_report(ptr, size, type, size > 8 || value_change,
smp_processor_id(), KCSAN_REPORT_RACE_SIGNAL);
} else if (value_change) {
/* Inferring a race, since the value should not have changed. */
kcsan_counter_inc(KCSAN_COUNTER_RACES_UNKNOWN_ORIGIN);
if (IS_ENABLED(CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN))
- kcsan_report(ptr, size, is_write, true,
+ kcsan_report(ptr, size, type, true,
smp_processor_id(),
KCSAN_REPORT_RACE_UNKNOWN_ORIGIN);
}
@@ -455,10 +456,10 @@ static __always_inline void check_access(const volatile void *ptr, size_t size,
*/
if (unlikely(watchpoint != NULL))
- kcsan_found_watchpoint(ptr, size, is_write, watchpoint,
+ kcsan_found_watchpoint(ptr, size, type, watchpoint,
encoded_watchpoint);
else if (unlikely(should_watch(ptr, type)))
- kcsan_setup_watchpoint(ptr, size, is_write);
+ kcsan_setup_watchpoint(ptr, size, type);
}
/* === Public interface ===================================================== */
diff --git a/kernel/kcsan/kcsan.h b/kernel/kcsan/kcsan.h
index d3b9a96ac8a4..8492da45494b 100644
--- a/kernel/kcsan/kcsan.h
+++ b/kernel/kcsan/kcsan.h
@@ -103,7 +103,7 @@ enum kcsan_report_type {
/*
* Print a race report from thread that encountered the race.
*/
-extern void kcsan_report(const volatile void *ptr, size_t size, bool is_write,
+extern void kcsan_report(const volatile void *ptr, size_t size, int access_type,
bool value_change, int cpu_id, enum kcsan_report_type type);
#endif /* _KERNEL_KCSAN_KCSAN_H */
diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c
index 0eea05a3135b..9f503ca2ff7a 100644
--- a/kernel/kcsan/report.c
+++ b/kernel/kcsan/report.c
@@ -24,7 +24,7 @@
static struct {
const volatile void *ptr;
size_t size;
- bool is_write;
+ int access_type;
int task_pid;
int cpu_id;
unsigned long stack_entries[NUM_STACK_ENTRIES];
@@ -41,8 +41,10 @@ static DEFINE_SPINLOCK(report_lock);
* Special rules to skip reporting.
*/
static bool
-skip_report(bool is_write, bool value_change, unsigned long top_frame)
+skip_report(int access_type, bool value_change, unsigned long top_frame)
{
+ const bool is_write = (access_type & KCSAN_ACCESS_WRITE) != 0;
+
if (IS_ENABLED(CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY) && is_write &&
!value_change) {
/*
@@ -63,9 +65,20 @@ skip_report(bool is_write, bool value_change, unsigned long top_frame)
return kcsan_skip_report_debugfs(top_frame);
}
-static inline const char *get_access_type(bool is_write)
+static const char *get_access_type(int type)
{
- return is_write ? "write" : "read";
+ switch (type) {
+ case 0:
+ return "read";
+ case KCSAN_ACCESS_ATOMIC:
+ return "read (marked)";
+ case KCSAN_ACCESS_WRITE:
+ return "write";
+ case KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ATOMIC:
+ return "write (marked)";
+ default:
+ BUG();
+ }
}
/* Return thread description: in task or interrupt. */
@@ -112,7 +125,7 @@ static int sym_strcmp(void *addr1, void *addr2)
/*
* Returns true if a report was generated, false otherwise.
*/
-static bool print_report(const volatile void *ptr, size_t size, bool is_write,
+static bool print_report(const volatile void *ptr, size_t size, int access_type,
bool value_change, int cpu_id,
enum kcsan_report_type type)
{
@@ -124,7 +137,7 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
/*
* Must check report filter rules before starting to print.
*/
- if (skip_report(is_write, true, stack_entries[skipnr]))
+ if (skip_report(access_type, true, stack_entries[skipnr]))
return false;
if (type == KCSAN_REPORT_RACE_SIGNAL) {
@@ -132,7 +145,7 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
other_info.num_stack_entries);
/* @value_change is only known for the other thread */
- if (skip_report(other_info.is_write, value_change,
+ if (skip_report(other_info.access_type, value_change,
other_info.stack_entries[other_skipnr]))
return false;
}
@@ -170,7 +183,7 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
switch (type) {
case KCSAN_REPORT_RACE_SIGNAL:
pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
- get_access_type(other_info.is_write), other_info.ptr,
+ get_access_type(other_info.access_type), other_info.ptr,
other_info.size, get_thread_desc(other_info.task_pid),
other_info.cpu_id);
@@ -181,14 +194,14 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
pr_err("\n");
pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
- get_access_type(is_write), ptr, size,
+ get_access_type(access_type), ptr, size,
get_thread_desc(in_task() ? task_pid_nr(current) : -1),
cpu_id);
break;
case KCSAN_REPORT_RACE_UNKNOWN_ORIGIN:
pr_err("race at unknown origin, with %s to 0x%px of %zu bytes by %s on cpu %i:\n",
- get_access_type(is_write), ptr, size,
+ get_access_type(access_type), ptr, size,
get_thread_desc(in_task() ? task_pid_nr(current) : -1),
cpu_id);
break;
@@ -223,7 +236,7 @@ static void release_report(unsigned long *flags, enum kcsan_report_type type)
* required for the report type, simply acquires report_lock and returns true.
*/
static bool prepare_report(unsigned long *flags, const volatile void *ptr,
- size_t size, bool is_write, int cpu_id,
+ size_t size, int access_type, int cpu_id,
enum kcsan_report_type type)
{
if (type != KCSAN_REPORT_CONSUMED_WATCHPOINT &&
@@ -243,7 +256,7 @@ static bool prepare_report(unsigned long *flags, const volatile void *ptr,
other_info.ptr = ptr;
other_info.size = size;
- other_info.is_write = is_write;
+ other_info.access_type = access_type;
other_info.task_pid = in_task() ? task_pid_nr(current) : -1;
other_info.cpu_id = cpu_id;
other_info.num_stack_entries = stack_trace_save(other_info.stack_entries, NUM_STACK_ENTRIES, 1);
@@ -302,14 +315,14 @@ static bool prepare_report(unsigned long *flags, const volatile void *ptr,
goto retry;
}
-void kcsan_report(const volatile void *ptr, size_t size, bool is_write,
+void kcsan_report(const volatile void *ptr, size_t size, int access_type,
bool value_change, int cpu_id, enum kcsan_report_type type)
{
unsigned long flags = 0;
kcsan_disable_current();
- if (prepare_report(&flags, ptr, size, is_write, cpu_id, type)) {
- if (print_report(ptr, size, is_write, value_change, cpu_id, type) && panic_on_warn)
+ if (prepare_report(&flags, ptr, size, access_type, cpu_id, type)) {
+ if (print_report(ptr, size, access_type, value_change, cpu_id, type) && panic_on_warn)
panic("panic_on_warn set ...\n");
release_report(&flags, type);
--
2.25.0.rc1.283.g88dfdc4193-goog
next prev parent reply other threads:[~2020-01-09 15:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-09 15:23 [PATCH -rcu 0/2] kcsan: Improvements to reporting Marco Elver
2020-01-09 15:23 ` Marco Elver [this message]
2020-01-09 15:23 ` [PATCH -rcu 2/2] kcsan: Rate-limit reporting per data races Marco Elver
2020-01-10 18:20 ` Marco Elver
2020-01-10 18:54 ` Marco Elver
2020-01-11 5:13 ` Paul E. McKenney
2020-01-09 16:27 ` [PATCH -rcu 0/2] kcsan: Improvements to reporting Paul E. McKenney
2020-01-09 17:03 ` Marco Elver
2020-01-09 17:31 ` Paul E. McKenney
2020-01-09 17:42 ` Marco Elver
2020-01-09 20:46 ` Paul E. McKenney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200109152322.104466-2-elver@google.com \
--to=elver@google.com \
--cc=andreyknvl@google.com \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).