| Message ID | f38b4333badbdabdb141d5ecc59518f50e5d3493.1696795837.git.lstoakes@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Abstract vma_merge() and split_vma() | expand |
On 10/8/23 22:23, Lorenzo Stoakes wrote: > Only in mmap_region() and copy_vma() do we add VMAs which occupy entirely > new regions of virtual memory. > > We can share the logic between these invocations and make it absolutely > explici to reduce confusion around the rather inscrutible parameters explicit ... inscrutable > possessed by vma_merge(). > > This also paves the way for a simplification of the core vma_merge() > implementation, as we seek to make the function entirely an implementation > detail. > > Note that on mmap_region(), vma fields are initialised to zero, so we can > simply reference these rather than explicitly specifying NULL. Right, if they were different from NULL, the code would be broken already. > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
On Mon, Oct 09, 2023 at 06:04:47PM +0200, Vlastimil Babka wrote: > On 10/8/23 22:23, Lorenzo Stoakes wrote: > > Only in mmap_region() and copy_vma() do we add VMAs which occupy entirely > > new regions of virtual memory. > > > > We can share the logic between these invocations and make it absolutely > > explici to reduce confusion around the rather inscrutible parameters > > explicit ... inscrutable > Ack will fix up in v2. > > possessed by vma_merge(). > > > > This also paves the way for a simplification of the core vma_merge() > > implementation, as we seek to make the function entirely an implementation > > detail. > > > > Note that on mmap_region(), vma fields are initialised to zero, so we can > > simply reference these rather than explicitly specifying NULL. > > Right, if they were different from NULL, the code would be broken already. > > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> > > Reviewed-by: Vlastimil Babka <vbabka@suse.cz> > Thanks!
diff --git a/mm/mmap.c b/mm/mmap.c index 58d71f84e917..51be864b876b 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2530,6 +2530,22 @@ struct vm_area_struct *vma_modify_uffd(struct vma_iterator *vmi, vma_policy(vma), new_ctx, anon_vma_name(vma)); } +/* + * Attempt to merge a newly mapped VMA with those adjacent to it. The caller + * must ensure that [start, end) does not overlap any existing VMA. + */ +static struct vm_area_struct *vma_merge_new_vma(struct vma_iterator *vmi, + struct vm_area_struct *prev, + struct vm_area_struct *vma, + unsigned long start, + unsigned long end, + pgoff_t pgoff) +{ + return vma_merge(vmi, vma->vm_mm, prev, start, end, vma->vm_flags, + vma->anon_vma, vma->vm_file, pgoff, vma_policy(vma), + vma->vm_userfaultfd_ctx, anon_vma_name(vma)); +} + /* * do_vmi_align_munmap() - munmap the aligned region from @start to @end. * @vmi: The vma iterator @@ -2885,10 +2901,9 @@ unsigned long mmap_region(struct file *file, unsigned long addr, * vma again as we may succeed this time. */ if (unlikely(vm_flags != vma->vm_flags && prev)) { - merge = vma_merge(&vmi, mm, prev, vma->vm_start, - vma->vm_end, vma->vm_flags, NULL, - vma->vm_file, vma->vm_pgoff, NULL, - NULL_VM_UFFD_CTX, NULL); + merge = vma_merge_new_vma(&vmi, prev, vma, + vma->vm_start, vma->vm_end, + pgoff); if (merge) { /* * ->mmap() can change vma->vm_file and fput @@ -3430,9 +3445,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, if (new_vma && new_vma->vm_start < addr + len) return NULL; /* should never get here */ - new_vma = vma_merge(&vmi, mm, prev, addr, addr + len, vma->vm_flags, - vma->anon_vma, vma->vm_file, pgoff, vma_policy(vma), - vma->vm_userfaultfd_ctx, anon_vma_name(vma)); + new_vma = vma_merge_new_vma(&vmi, prev, vma, addr, addr + len, pgoff); if (new_vma) { /* * Source vma may have been merged into new_vma
Only in mmap_region() and copy_vma() do we add VMAs which occupy entirely new regions of virtual memory. We can share the logic between these invocations and make it absolutely explici to reduce confusion around the rather inscrutible parameters possessed by vma_merge(). This also paves the way for a simplification of the core vma_merge() implementation, as we seek to make the function entirely an implementation detail. Note that on mmap_region(), vma fields are initialised to zero, so we can simply reference these rather than explicitly specifying NULL. Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> --- mm/mmap.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)