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

[Wei Yang <richard.weiyang@gmail.com>]

On Mon, Jan 27, 2025 at 10:50:17AM +0000, Lorenzo Stoakes wrote:
>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.
>

Yes I remember your suggestion. I send this because it is a bug fix to me. 
Per my understanding on your word, it is ok to send a fix.

If I misunderstand, I apologize.

>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.
>

You are right. I will try to be better next time.

As you mentioned a query before sending a patch, this is preferred, right?
Hope I don't mess this again.

>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.
>

Thanks, I would be more considerate next time.

>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.
>

Thanks to you and Liam. I will try to do better to not waste your time.

>>
>> 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
>>
>>

-- 
Wei Yang
Help you, Help me