From: Marco Elver <elver@google.com>
To: elver@google.com, paulmck@kernel.org
Cc: mark.rutland@arm.com, will@kernel.org, dvyukov@google.com,
glider@google.com, boqun.feng@gmail.com,
kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org
Subject: [PATCH 7/9] kcsan: Remove kcsan_report_type
Date: Wed, 14 Apr 2021 13:28:23 +0200 [thread overview]
Message-ID: <20210414112825.3008667-8-elver@google.com> (raw)
In-Reply-To: <20210414112825.3008667-1-elver@google.com>
From: Mark Rutland <mark.rutland@arm.com>
Now that the reporting code has been refactored, it's clear by
construction that print_report() can only be passed
KCSAN_REPORT_RACE_SIGNAL or KCSAN_REPORT_RACE_UNKNOWN_ORIGIN, and these
can also be distinguished by the presence of `other_info`.
Let's simplify things and remove the report type enum, and instead let's
check `other_info` to distinguish these cases. This allows us to remove
code for cases which are impossible and generally makes the code simpler.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
[ elver@google.com: add updated comments to kcsan_report_*() functions ]
Signed-off-by: Marco Elver <elver@google.com>
---
kernel/kcsan/kcsan.h | 33 +++++++++++++--------------------
kernel/kcsan/report.c | 29 +++++++----------------------
2 files changed, 20 insertions(+), 42 deletions(-)
diff --git a/kernel/kcsan/kcsan.h b/kernel/kcsan/kcsan.h
index 2ee43fd5d6a4..572f119a19eb 100644
--- a/kernel/kcsan/kcsan.h
+++ b/kernel/kcsan/kcsan.h
@@ -116,32 +116,25 @@ enum kcsan_value_change {
KCSAN_VALUE_CHANGE_TRUE,
};
-enum kcsan_report_type {
- /*
- * The thread that set up the watchpoint and briefly stalled was
- * signalled that another thread triggered the watchpoint.
- */
- KCSAN_REPORT_RACE_SIGNAL,
-
- /*
- * A thread found and consumed a matching watchpoint.
- */
- KCSAN_REPORT_CONSUMED_WATCHPOINT,
-
- /*
- * No other thread was observed to race with the access, but the data
- * value before and after the stall differs.
- */
- KCSAN_REPORT_RACE_UNKNOWN_ORIGIN,
-};
-
/*
- * Notify the report code that a race occurred.
+ * The calling thread hit and consumed a watchpoint: set the access information
+ * to be consumed by the reporting thread. No report is printed yet.
*/
void kcsan_report_set_info(const volatile void *ptr, size_t size, int access_type,
int watchpoint_idx);
+
+/*
+ * The calling thread observed that the watchpoint it set up was hit and
+ * consumed: print the full report based on information set by the racing
+ * thread.
+ */
void kcsan_report_known_origin(const volatile void *ptr, size_t size, int access_type,
enum kcsan_value_change value_change, int watchpoint_idx);
+
+/*
+ * No other thread was observed to race with the access, but the data value
+ * before and after the stall differs. Reports a race of "unknown origin".
+ */
void kcsan_report_unknown_origin(const volatile void *ptr, size_t size, int access_type);
#endif /* _KERNEL_KCSAN_KCSAN_H */
diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c
index ba924f110c95..50cee2357885 100644
--- a/kernel/kcsan/report.c
+++ b/kernel/kcsan/report.c
@@ -326,7 +326,6 @@ static void print_verbose_info(struct task_struct *task)
}
static void print_report(enum kcsan_value_change value_change,
- enum kcsan_report_type type,
const struct access_info *ai,
const struct other_info *other_info)
{
@@ -343,7 +342,7 @@ static void print_report(enum kcsan_value_change value_change,
if (skip_report(KCSAN_VALUE_CHANGE_TRUE, stack_entries[skipnr]))
return;
- if (type == KCSAN_REPORT_RACE_SIGNAL) {
+ if (other_info) {
other_skipnr = get_stack_skipnr(other_info->stack_entries,
other_info->num_stack_entries);
other_frame = other_info->stack_entries[other_skipnr];
@@ -358,8 +357,7 @@ static void print_report(enum kcsan_value_change value_change,
/* Print report header. */
pr_err("==================================================================\n");
- switch (type) {
- case KCSAN_REPORT_RACE_SIGNAL: {
+ if (other_info) {
int cmp;
/*
@@ -371,22 +369,15 @@ static void print_report(enum kcsan_value_change value_change,
get_bug_type(ai->access_type | other_info->ai.access_type),
(void *)(cmp < 0 ? other_frame : this_frame),
(void *)(cmp < 0 ? this_frame : other_frame));
- } break;
-
- case KCSAN_REPORT_RACE_UNKNOWN_ORIGIN:
+ } else {
pr_err("BUG: KCSAN: %s in %pS\n", get_bug_type(ai->access_type),
(void *)this_frame);
- break;
-
- default:
- BUG();
}
pr_err("\n");
/* Print information about the racing accesses. */
- switch (type) {
- case KCSAN_REPORT_RACE_SIGNAL:
+ if (other_info) {
pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
get_access_type(other_info->ai.access_type), other_info->ai.ptr,
other_info->ai.size, get_thread_desc(other_info->ai.task_pid),
@@ -404,16 +395,10 @@ static void print_report(enum kcsan_value_change value_change,
pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
get_access_type(ai->access_type), ai->ptr, ai->size,
get_thread_desc(ai->task_pid), ai->cpu_id);
- break;
-
- case KCSAN_REPORT_RACE_UNKNOWN_ORIGIN:
+ } else {
pr_err("race at unknown origin, with %s to 0x%px of %zu bytes by %s on cpu %i:\n",
get_access_type(ai->access_type), ai->ptr, ai->size,
get_thread_desc(ai->task_pid), ai->cpu_id);
- break;
-
- default:
- BUG();
}
/* Print stack trace of this thread. */
stack_trace_print(stack_entries + skipnr, num_stack_entries - skipnr,
@@ -623,7 +608,7 @@ void kcsan_report_known_origin(const volatile void *ptr, size_t size, int access
* be done once we know the full stack trace in print_report().
*/
if (value_change != KCSAN_VALUE_CHANGE_FALSE)
- print_report(value_change, KCSAN_REPORT_RACE_SIGNAL, &ai, other_info);
+ print_report(value_change, &ai, other_info);
release_report(&flags, other_info);
out:
@@ -640,7 +625,7 @@ void kcsan_report_unknown_origin(const volatile void *ptr, size_t size, int acce
lockdep_off(); /* See kcsan_report_known_origin(). */
raw_spin_lock_irqsave(&report_lock, flags);
- print_report(KCSAN_VALUE_CHANGE_TRUE, KCSAN_REPORT_RACE_UNKNOWN_ORIGIN, &ai, NULL);
+ print_report(KCSAN_VALUE_CHANGE_TRUE, &ai, NULL);
raw_spin_unlock_irqrestore(&report_lock, flags);
lockdep_on();
--
2.31.1.295.g9ea45b61b8-goog
next prev parent reply other threads:[~2021-04-14 11:29 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-14 11:28 [PATCH 0/9] kcsan: Add support for reporting observed value changes Marco Elver
2021-04-14 11:28 ` [PATCH 1/9] kcsan: Simplify value change detection Marco Elver
2021-04-14 11:28 ` [PATCH 2/9] kcsan: Distinguish kcsan_report() calls Marco Elver
2021-04-14 11:28 ` [PATCH 3/9] kcsan: Refactor passing watchpoint/other_info Marco Elver
2021-04-14 11:28 ` [PATCH 4/9] kcsan: Fold panic() call into print_report() Marco Elver
2021-04-14 11:28 ` [PATCH 5/9] kcsan: Refactor access_info initialization Marco Elver
2021-04-14 11:28 ` [PATCH 6/9] kcsan: Remove reporting indirection Marco Elver
2021-04-14 11:28 ` Marco Elver [this message]
2021-04-14 11:28 ` [PATCH 8/9] kcsan: Report observed value changes Marco Elver
2021-04-14 11:28 ` [PATCH 9/9] kcsan: Document "value changed" line Marco Elver
2021-04-15 11:47 ` [PATCH 0/9] kcsan: Add support for reporting observed value changes Mark Rutland
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=20210414112825.3008667-8-elver@google.com \
--to=elver@google.com \
--cc=boqun.feng@gmail.com \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=paulmck@kernel.org \
--cc=will@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).