Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 20 Apr 2022 11:04:07 +0200
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
Message-ID: <Yl/Mh4gjG1hYW2nA@elver.google.com>
References: <20220416081355.2155050-1-jcmvbkbc@gmail.com>
 <CANpmjNNW0kLf2Ou6i_dNeRLO=Qrru4bOEfJ=be=Dfig4wnQ67g@mail.gmail.com>
 <CAMo8BfJM0JHqh8Nz3LuK7Ccu7WB1Cup0mX+RYvO1yft_K4hyLQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMo8BfJM0JHqh8Nz3LuK7Ccu7WB1Cup0mX+RYvO1yft_K4hyLQ@mail.gmail.com>
User-Agent: Mutt/2.1.4 (2021-12-11)
Precedence: bulk

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