On 2024/2/19 07:03, Lorenzo Stoakes wrote:
On Sun, Feb 18, 2024 at 04:50:28PM +0800, Yajun Deng wrote:
If the prev vma exists and the end is less than the end of prev, we
can return NULL immediately. This reduces unnecessary operations.
Signed-off-by: Yajun Deng <yajun.deng@xxxxxxxxx>
Adding Vlastimil, while get_maintainers.pl might not show it very clearly,
myself, Vlastimil and Liam often work with vma_merge() so it's handy to cc
us on these if you can!
Okay.
---
mm/mmap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index 8f176027583c..b738849321c0 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -827,7 +827,7 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags,
*
* **** **** ****
* PPPPPPNNNNNN PPPPPPNNNNNN PPPPPPCCCCCC
- * cannot merge might become might become
+ * cannot merge 9 might become might become
While I welcome your interest here :) I am not a fan of the 'case' approach
to this function as-is and plan to heavily refactor this when I get a chance.
But at any rate, an early-exit situation is not a merge case, merge cases
describe cases where we _can_ merge, so we can drop this case 9 stuff (this
is not your fault, it's understandable why you would label this, this
function is just generally unclear).
Yes, it's not a merge case. I label this to make it easier to understand.
* PPNNNNNNNNNN PPPPPPPPPPCC
* mmap, brk or case 4 below case 5 below
* mremap move:
@@ -890,6 +890,9 @@ static struct vm_area_struct
if (vm_flags & VM_SPECIAL)
return NULL;
+ if (prev && end < prev->vm_end) /* case 9 */
+ return NULL;
+
I need to get back into vma_merge() head space, but I don't actually think
a caller that's behaving correctly should ever do this. I know the ASCII
diagram above lists it as a thing that can happen, but I think we
implicitly avoid this from the way we invoke callers. Either prev == vma as
per vma_merge_extend(), or the loops that invoke vma_merge_new_vma()
wouldn't permit this to occur.
No, it will actually happen. That's why I submitted this patch.
Let me look into it more deeply + reply again a bit later, I mean we could
perhaps do with asserting this somehow, but I don't think it's useful to do
an early exit for something that ostensibly _shouldn't_ happen.
/* Does the input range span an existing VMA? (cases 5 - 8) */
curr = find_vma_intersection(mm, prev ? prev->vm_end : 0, end);
--
2.25.1
