Message-ID: <20221024155357.GZ5600@paulmck-ThinkPad-P17-Gen-1>
Date:   Mon, 24 Oct 2022 08:53:57 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Willy Tarreau <w@....eu>
Cc:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selftests/nolibc: add 7 tests for memcmp()

On Sat, Oct 22, 2022 at 01:22:28PM +0200, Willy Tarreau wrote:
> On Fri, Oct 21, 2022 at 11:00:40AM -0700, Paul E. McKenney wrote:
> > > It's even easier, you don't even need the clean phase in include/nolibc.
> > > I'm doing this and it's sufficient:
> > > 
> > >   make -C tools/testing/selftests/nolibc clean
> > >   make -C tools/testing/selftests/nolibc nolibc-test
> > >   tools/testing/selftests/nolibc/nolibc-test
> > > 
> > > Or for the test under QEMU, which involves a kernel build:
> > > 
> > >   make -C tools/testing/selftests/nolibc clean
> > >   make -C tools/testing/selftests/nolibc -j $(nproc) run
> > > 
> > > Where would you first look for such a hint ? Maybe the help output of
> > > the default "make" command could send as a hint that a clean is needed
> > > after patching nolibc and that could be sufficient ? I just want to make
> > > sure users don't waste their time trying to find what they could be doing
> > > wrong.
> > 
> > Maybe it suffices for the near term for me to put this information in
> > the signed tag for the pull request?
> 
> It can be sufficient for short term indeed, but it can be easy as well
> for me to mention it in the make output.

Why not both?  ;-)

> > Another approach would be to remind about "make clean" in the case of
> > a test failure.  Or make test failure combined with a detected change
> > trigger an automatic "make clean" and a retry.
> 
> In fact failures are not the only case. For me it was the opposite. I
> applied Rasmus' fix, then I developed the test, verified that it worked,
> then reverted Rasmus' fix... to find that the test didn't catch the
> failure. I had a second look at the original patch and figured that
> the -192..+192 values were really not possible with a char so I
> concluded that a clean was needed. But leaving something in a claimed
> working state while it's not can be sufficiently misleading and make
> one waste significant time, because in such cases we rarely search why
> it works.

Fair point!  False negatives can be quite annoying as well.

> > Or other schemes of increasing complexity and fragility.  ;-)
> 
> That's exactly what I'd like to avoid with such a lightweight component.
> If it takes more time to figure why something is going wrong than to
> write a test, we'll all give up. I think that a clean for QEMU is worth
> it because the kernel is rebuilt and its dependencies are quite robust,
> so that one would be a surprise. For other tests, probably leaving it
> explicit with a hint that it's needed should suffice. I'll recheck what
> conditions the installation of uapi headers because that's really what
> I don't want to see happening all the time. The rest is discrete, it's
> just a few files being copied, maybe it can be done every time.
> 
> Will keep thinking about it and hopefully propose a patch to make the
> tests easier to use before we're too far in the 6.1 release.

Another possibility is to have a separate developers' and maintainers'
option.  Linus and I do "make whatever" for some value of "whatever"
that builds from scratch, doing whatever cleaning that might be required.
Developers use targets that are faster but have the possibility of false
positives and false negatives.

But maybe you have something better in mind.

> Thanks for keeping the conversation flowing, that helps me!

Looking forward to seeing what you come up with!

							Thanx, Paul

Powered by blists - more mailing lists