MIME-Version: 1.0
References: <20230301143933.2374658-1-glider@google.com> <20230301143933.2374658-2-glider@google.com>
 <CANpmjNOG=T8R=BXO8PUX3FJQnKQfPjNyLGJ0wG5G_4_mHwJ-gA@mail.gmail.com>
In-Reply-To: <CANpmjNOG=T8R=BXO8PUX3FJQnKQfPjNyLGJ0wG5G_4_mHwJ-gA@mail.gmail.com>
From:   Alexander Potapenko <glider@google.com>
Date:   Thu, 2 Mar 2023 15:48:29 +0100
Message-ID: <CAG_fn=X_E7r7JnBKWTygwiTa7HWJ1=AhtJOoH7is_mz0fhgfXA@mail.gmail.com>
Subject: Re: [PATCH 2/4] kmsan: another take at fixing memcpy tests
To:     Marco Elver <elver@google.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, x86@kernel.org,
        dave.hansen@linux.intel.com, hpa@zytor.com,
        akpm@linux-foundation.org, dvyukov@google.com, nathan@kernel.org,
        ndesaulniers@google.com, kasan-dev@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> > +#define DO_NOT_OPTIMIZE(var) asm("" ::: "memory")
>
> That's just a normal "barrier()" - use that instead?

Ok, will do (I still think I'd better hide it behind a macro so that
we can change the implementation of DO_NOT_OPTIMIZE in the future if
the compiler starts outsmarting us again.

> > +/*
> > + * Test case: ensure that memcpy() correctly copies initialized values.
> > + */
> > +static void test_init_memcpy(struct kunit *test)
> > +{
> > +       EXPECTATION_NO_REPORT(expect);
> > +       volatile int src;
> > +       volatile int dst = 0;
> > +
> > +       // Ensure DO_NOT_OPTIMIZE() does not cause extra checks.
>
> ^^ this comment seems redundant now, given DO_NOT_OPTIMIZE() has a
> comment (it's also using //-style comment).

Moved it to the test description:

/*
 * Test case: ensure that memcpy() correctly copies initialized values.
 * Also serves as a regression test to ensure DO_NOT_OPTIMIZE() does not cause
 * extra checks.
 */

I think it's still relevant here.