* Yajun Deng <yajun.deng@xxxxxxxxx> [240222 03:56]:
...
Oh, yes. Test on the platform that expands upwards would be best.Yes, VMA_ITERATOR can only pass one address.This is confusing. I think you are doing this so that the vma iterator@@ -1959,11 +1958,12 @@ static int expand_upwards(struct vm_area_struct *vma, unsigned long address)
struct vm_area_struct *next;
unsigned long gap_addr;
int error = 0;
- MA_STATE(mas, &mm->mm_mt, vma->vm_start, address);
+ VMA_ITERATOR(vmi, mm, 0);
if (!(vma->vm_flags & VM_GROWSUP))
return -EFAULT;
+ vma_iter_config(&vmi, vma->vm_start, address);
is set up the same as the maple state, and not what is logically
necessary?
Okay, I will create a new helper function for this in the mm/internal.h.This isn't really hiding the maple state./* Guard against exceeding limits of the address space. */
address &= PAGE_MASK;
if (address >= (TASK_SIZE & PAGE_MASK))
@@ -1985,15 +1985,15 @@ static int expand_upwards(struct vm_area_struct *vma, unsigned long address)
}
if (next)
- mas_prev_range(&mas, address);
+ mas_prev_range(&vmi.mas, address);
The above maple state changes is to get the maple state to point to the- __mas_set_range(&mas, vma->vm_start, address - 1);
- if (mas_preallocate(&mas, vma, GFP_KERNEL))
+ vma_iter_config(&vmi, vma->vm_start, address);
correct area for the preallocation call below. This seems unnecessary
to me.
We really should just set it up correctly. Unfortunately, with the VMA
iterator, that's not really possible on initialization.
What we can do is use the vma->vm_start for the initialization, then use
vma_iter_config() here. That will not reset any state - but that's fine
because the preallocation is the first call that actually uses it
anyways.
So we can initialize with vma->vm_start, don't call vma_iter_config
until here, and also drop the if (next) part.
This is possible here because it's not optimised like the
expand_upwards() case, which uses the state to check prev and avoids an
extra walk.
Please make sure to test with the ltp tests on the stack combining, etc
on a platform that expands down.
It seems something wrong about this description. This change is in
expand_upwards(), but not in
expand_downwards(). So we should test it on a platform that expands up.
Sorry about the mix up.
AndYes, I think the if (next) part is unnecessary because the maple
drop the if (next) part
is unnecessary. Did I get that right?
state/vma iterator has not actually moved - we use
find_vma_intersection() to locate next and not the iterator. This is
different than what we do in the expand_downwards.
Note that, in the even that we reach the limit and cannot return a
usable address, these functions will call the counterpart and search in
the opposite direction.
...Okay, I will test it.Testing this can be tricky. Thanks for looking at it.
Thanks,
Liam