Re: [PATCH v1 2/2] selftests/mm: mseal_test add more tests

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Thu, Aug 29, 2024 at 08:30:11AM GMT, Jeff Xu wrote:
> Hi Lorenzo
>
> On Thu, Aug 29, 2024 at 8:14 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Thu, Aug 29, 2024 at 07:45:56AM GMT, Jeff Xu wrote:
> > > HI Andrew
> > >
> > > On Wed, Aug 28, 2024 at 3:55 PM <jeffxu@chromium.org> wrote:
> > > >
> > > > From: Jeff Xu <jeffxu@chromium.org>
> > > >
> > > > Add more testcases and increase test coverage, e.g. add
> > > > get_vma_size to check VMA size and prot bits.
> >
> > This commit message is ridiculously short for such a massive change, even for
> > test code.
> >
> > > >
> > >
> > > Could you please pull the self-test part of this patch series to mm-unstable ?
> > > It will help to prevent regression.
> >
> > No, please don't.
> >
> > This needs review.
> >
> > These tests establish a precedent as to how mseal should behave, this is
> > something that needs community review, not to just be taken.
> >
> > There's already been a great deal of confusion/contentious discussion
> > around mseal() and its implementation.
> >
> > Pushing in ~800 lines of test code asserting how mseal() should behave
> > without review isn't helping things.
> >
> > Also, this is a really unusual way to send a series - why is this a 2/2 in
> > reply to the 1/2 and no cover letter? Why is this change totally unrelated
> > to the other patch?
> >
> > Can you send this as a separate patch, preferably as an RFC so we can
> > ensure that we all agree on how mseal() should behave?
> >
> > Sorry to be contentious here, but I think we need to find a more
> > constructive, collaborative way forward with mseal() and to act with a
> > little more caution, given the problems that the original series has caused
> > I'd think this is in the best interests of all.
> >
> > Thanks for understanding!
> >
> There have been two bugs I found recently on mseal.
> One during V2 of in-loop change and the other mentioned in 1/2 of this patch.
>

Jeff you've ignored pretty much everything I've said here. This is not
collaboration. And you keep doing this + causing disharmony among other
devleopers. It's getting tiresome, and you need to do better.

If you insist on review for this patch as it stands - NACK.

The commit message is ludicriously short, you've not sent the series
correctly, and you are ignoring feedback.

Resend this with a substantially improved commit message and ideally some
actual comments in your tests rather than a giant lump of code which
constitutes 'how Jeff feels mseal() should work'.

Then when people give feedback - listen.