On Tue, Jan 28, 2025 at 01:56:19AM +0000, Wei Yang wrote: > 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. This is not a bug. A bug is something that breaks the kernel. Having to use a different gap in the vastness of virtual address space is not a bug. This is you misunderstanding unmapped_area() as a function that for some reason in your view must function identically in being minimally conservative whether top-down or bottom-up including scenarios in which start_gap is an impossible value. As far as I can tell the function is behaving completely correctly, it is just conservative in accounting for worst-case alignment and front and rear gaps of appropriate size. You could have more civilly tested out this theory, rather than wasting presumably a LOT of your time on this (meanwhile taking -no- time to write commit messages), simply asking Liam about it on-list. Communication is key. > > 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 >
