mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Max Filippov <jcmvbkbc@gmail.com>
Cc: "open list:TENSILICA XTENSA PORT (xtensa)" 
	<linux-xtensa@linux-xtensa.org>, Chris Zankel <chris@zankel.net>,
	LKML <linux-kernel@vger.kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	kasan-dev@googlegroups.com
Subject: Re: [PATCH] xtensa: enable KCSAN
Date: Wed, 20 Apr 2022 11:04:07 +0200	[thread overview]
Message-ID: <Yl/Mh4gjG1hYW2nA@elver.google.com> (raw)
In-Reply-To: <CAMo8BfJM0JHqh8Nz3LuK7Ccu7WB1Cup0mX+RYvO1yft_K4hyLQ@mail.gmail.com>

On Tue, Apr 19, 2022 at 07:59PM -0700, Max Filippov wrote:
[...]
> > The stubs are the only thing I don't understand. More elaboration on
> > why this is required would be useful (maybe there's another way to
> > solve?).
> 
> It doesn't build without it, because the compiler left function calls
> in the code:
> 
> xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
> `__tsan_atomic32_compare_exchange_val':
> kernel/kcsan/core.c:1262: undefined reference to `__atomic_load_8'
> xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
> `__tsan_atomic64_load':
[...]
> 
> None of these functions are called because xtensa doesn't have
> 64-bit atomic ops.
> 
> I guess that another way to fix it would be making
> DEFINE_TSAN_ATOMIC_OPS(64);
> conditional and not enabling it when building for xtensa.

I see - however, it seems the kernel provides 64-bit atomics to xtensa
using lib/atomic64.c:

	arch/xtensa/Kconfig:    select GENERIC_ATOMIC64

So the right thing to do might be to implement the builtin atomics using
the kernel's atomic64_* primitives. However, granted, the builtin
atomics might not be needed on xtensa (depending on configuration).
Their existence is due to some compiler instrumentation emitting
builtin-atomics (Clang's GCOV), folks using them accidentally and
blaming KCSAN (also https://paulmck.livejournal.com/64970.html).

So I think it's fair to leave them to BUG() until somebody complains (at
which point they need to be implemented). I leave it to you.

> > > Disable KCSAN instrumentation in arch/xtensa/boot.
> >
> > Given you went for barrier instrumentation, I assume you tested with a
> > CONFIG_KCSAN_STRICT=y config?
> 
> Yes.
> 
> > Did the kcsan_test pass?
> 
> current results are the following on QEMU:
> 
>      # test_missing_barrier: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1313
>      Expected match_expect to be true, but is false
>      # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1356
>      Expected match_expect to be true, but is false
>  # kcsan: pass:27 fail:2 skip:0 total:29
>  # Totals: pass:193 fail:4 skip:0 total:197
> 
> and the following on the real hardware:
> 
>     # test_concurrent_races: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:762
>     Expected match_expect to be true, but is false
>     # test_write_write_struct_part: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:910
>     Expected match_expect to be true, but is false
>     # test_assert_exclusive_access_writer: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1077
>     Expected match_expect_access_writer to be true, but is false
>     # test_assert_exclusive_bits_change: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1098
>     Expected match_expect to be true, but is false
>     # test_assert_exclusive_writer_scoped: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1136
>     Expected match_expect_start to be true, but is false
>     # test_missing_barrier: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:1313
>     Expected match_expect to be true, but is false
>     # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1356
>     Expected match_expect to be true, but is false
> # kcsan: pass:22 fail:7 skip:0 total:29
> # Totals: pass:177 fail:20 skip:0 total:197

Each test case is run with varying number of threads - am I correctly
inferring that out of all test cases, usually only one such run failed,
and runs with different number of threads (of the same test case)
succeeded?

If that's the case, I think we can say that it works, and the failures
are due to flakiness with either higher or lower threads counts. I know
that some test cases might still be flaky under QEMU TCG because of how
it does concurrent execution of different CPU cores.

Thanks,
-- Marco

  reply	other threads:[~2022-04-20  9:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-16  8:13 Max Filippov
2022-04-19 10:15 ` Marco Elver
2022-04-20  2:59   ` Max Filippov
2022-04-20  9:04     ` Marco Elver [this message]
2022-04-20 18:11       ` Max Filippov
2022-04-20 22:58         ` Marco Elver

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=Yl/Mh4gjG1hYW2nA@elver.google.com \
    --to=elver@google.com \
    --cc=chris@zankel.net \
    --cc=dvyukov@google.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.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

all inboxes | Powered by JetHome®