Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200505182821.47708-1-elver@google.com> <CABVgOSmg8z1TpMh7NPy0M+9Gs2JT097-j_XGBRGhKk_3y2J-oA@mail.gmail.com>
In-Reply-To: <CABVgOSmg8z1TpMh7NPy0M+9Gs2JT097-j_XGBRGhKk_3y2J-oA@mail.gmail.com>
From:   Marco Elver <elver@google.com>
Date:   Wed, 6 May 2020 12:26:25 +0200
Message-ID: <CANpmjNOF5mPzU+TT+LftpfLFEhWUy1s1To1rsPXZBORTgfM_fw@mail.gmail.com>
Subject: Re: [PATCH v2] kcsan: Add test suite
To:     David Gow <davidgow@google.com>
Cc:     "Paul E. McKenney" <paulmck@kernel.org>,
        Dmitry Vyukov <dvyukov@google.com>,
        Alexander Potapenko <glider@google.com>,
        Andrey Konovalov <andreyknvl@google.com>,
        kasan-dev <kasan-dev@googlegroups.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        KUnit Development <kunit-dev@googlegroups.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 6 May 2020 at 06:45, David Gow <davidgow@google.com> wrote:
>
> On Wed, May 6, 2020 at 2:30 AM Marco Elver <elver@google.com> wrote:
> >
> > This adds KCSAN test focusing on behaviour of the integrated runtime.
> > Tests various race scenarios, and verifies the reports generated to
> > console. Makes use of KUnit for test organization, and the Torture
> > framework for test thread control.
> >
> > Signed-off-by: Marco Elver <elver@google.com>
>
> Thanks, this works much better on my setup: having an explicit error
> for there not being enough CPUs is a lot better than hanging. It'd
> still be nice to have these be "skipped" rather than "failed" at some
> stage, but that's a nice-to-have for the future once we've implemented
> such a thing in KUnit.

Will keep an eye on KUnit adding support for this, and in future we
can change it. Although I'd argue that these tests failing is a signal
that a particular KCSAN based CI setup isn't terribly useful at
finding data races, which can still be a valuable signal to have.

> I'm still a little hesitant about non-deterministic tests in general =E2=
=80=94
> even if they're only run when CONFIG_KCSAN is enabled, it's possible
> that a future CI system could run under KCSAN and report false
> breakages on unrelated patches. Given no such setup exists yet,
> though, I think it's probably a problem for the future rather than a
> blocker at the moment.

True. But as noted above, it might also highlight an issue with the CI
system's ability to detect data races if KCSAN is enabled, which is
the whole point of having a KCSAN test setup. But yes, let's cross
that bridge when such a system actually exists.

> Regardless, I hit no unexpected issues in my testing, so,
>
> Tested-by: David Gow <davidgow@google.com>

Thank you for testing!

-- Marco