Re: [PATCH v2 1/8] kcsan: Add Kernel Concurrency Sanitizer infrastructure

From: Oleg Nesterov
Date: Tue Oct 22 2019 - 11:49:20 EST


On 10/17, Marco Elver wrote:
>
> + /*
> + * Delay this thread, to increase probability of observing a racy
> + * conflicting access.
> + */
> + udelay(get_delay());
> +
> + /*
> + * Re-read value, and check if it is as expected; if not, we infer a
> + * racy access.
> + */
> + switch (size) {
> + case 1:
> + is_expected = expect_value._1 == READ_ONCE(*(const u8 *)ptr);
> + break;
> + case 2:
> + is_expected = expect_value._2 == READ_ONCE(*(const u16 *)ptr);
> + break;
> + case 4:
> + is_expected = expect_value._4 == READ_ONCE(*(const u32 *)ptr);
> + break;
> + case 8:
> + is_expected = expect_value._8 == READ_ONCE(*(const u64 *)ptr);
> + break;
> + default:
> + break; /* ignore; we do not diff the values */
> + }
> +
> + /* Check if this access raced with another. */
> + if (!remove_watchpoint(watchpoint)) {
> + /*
> + * No need to increment 'race' counter, as the racing thread
> + * already did.
> + */
> + kcsan_report(ptr, size, is_write, smp_processor_id(),
> + kcsan_report_race_setup);
> + } else if (!is_expected) {
> + /* Inferring a race, since the value should not have changed. */
> + kcsan_counter_inc(kcsan_counter_races_unknown_origin);
> +#ifdef CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN
> + kcsan_report(ptr, size, is_write, smp_processor_id(),
> + kcsan_report_race_unknown_origin);
> +#endif
> + }

Not sure I understand this code...

Just for example. Suppose that task->state = TASK_UNINTERRUPTIBLE, this task
does __set_current_state(TASK_RUNNING), another CPU does wake_up_process(task)
which does the same UNINTERRUPTIBLE -> RUNNING transition.

Looks like, this is the "data race" according to kcsan?

Hmm. even the "if (!(p->state & state))" check in try_to_wake_up() can trigger
kcsan_report() ?

Oleg.