Re: [PATCH 0/2] vma: fix unmapped_area()

Lorenzo Stoakes
2025-01-27T10:51:06Z

Hi Wei,

I seem to recall us having a very recent converation about holding off on
patches like these for a little while to which you agreed, and then you
sent this pretty much the very next day? And during the merge window?
Honestly not _hugely_ impressed with that.

In my view this patch should have instead started as a query to Liam about
the gap calculation, this would have been far more civil and would have
allowed us to determine for sure if the approach you've taken here is
valid.

Given your history of sending entirely trivial patches which we keep asking
you not to send (mixed in with the occasional actually useful patch) it is
KEY to communicate to ensure we're on the same page.

If you send meaningful commits, we want to merge them. Arbitrarily sending
something like this, at this point in time, when you've been asked not to -
does not help achieve this aim.

On Mon, Jan 27, 2025 at 07:55:25AM +0000, Wei Yang wrote:
> The gap check in unmapped_area() seems not correct.
>
> Add test cases to verify the behavior.

This is an -entirely- unacceptable cover letter. It's two lines dude. Give
some details. You're actually tackling a very, very specific aspect and
scenario in some of the most sensitive code in all of mm.

You really, really need to be clear on what it is you're doing, why, what
workload you were doing to hit this, what testing you've done, what real
life things this interacts with etc. etc.

It makes our lives easier as maintainers. Right now I see this as 'another
trivial Wei patch', you need to provide details to prove otherwise, if that
is indeed, not the case.

Also your subject line here is horrible - 'fix unmapped_area()' - actually
you seem to be (in your view) correcting the calculation with respect to
upward-growing stacks. Correct me if I'm wrong. I mean even your patch 1/2
has a better message... It needs to be more specific to what you're doing.

>
> Wei Yang (2):
> mm/vma: fix gap check for unmapped_area with VM_GROWSDOWN
> tools: testing: add unmapped_area() tests
>
> mm/vma.c | 2 +-
> tools/testing/vma/vma.c | 177 +++++++++++++++++++++++++++++++
> tools/testing/vma/vma_internal.h | 2 +-
> 3 files changed, 179 insertions(+), 2 deletions(-)
>
> --
> 2.34.1
>
>