Skip to content

Commit

Permalink
kcsan: Remove kcsan_report_type
Browse files Browse the repository at this point in the history
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>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
  • Loading branch information
Mark Rutland authored and Paul E. McKenney committed May 18, 2021
1 parent 19dfdc0 commit 609f809
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 42 deletions.
33 changes: 13 additions & 20 deletions kernel/kcsan/kcsan.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
29 changes: 7 additions & 22 deletions kernel/kcsan/report.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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];
Expand All @@ -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;

/*
Expand All @@ -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),
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand All @@ -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();
Expand Down

0 comments on commit 609f809

Please sign in to comment.