Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Mon, 9 Oct 2023 19:22:07 +0100
From:   Lorenzo Stoakes <lstoakes@gmail.com>
To:     Vlastimil Babka <vbabka@suse.cz>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <brauner@kernel.org>,
        "=Liam R . Howlett" <Liam.Howlett@oracle.com>,
        linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 4/4] mm: abstract VMA extension and merge into
 vma_merge_extend() helper
Message-ID: <ba75d32d-521a-4c8d-8b4c-fa491131e262@lucifer.local>
References: <cover.1696795837.git.lstoakes@gmail.com>
 <1ed3d1ba0069104e1685298aa2baf980c38a85ff.1696795837.git.lstoakes@gmail.com>
 <fe147a1e-6fe5-309f-b2e7-48f5f3c97bae@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <fe147a1e-6fe5-309f-b2e7-48f5f3c97bae@suse.cz>
Precedence: bulk

On Mon, Oct 09, 2023 at 06:30:02PM +0200, Vlastimil Babka wrote:
> On 10/8/23 22:23, Lorenzo Stoakes wrote:
> > mremap uses vma_merge() in the case where a VMA needs to be extended. This
> > can be significantly simplified and abstracted.
> >
> > This makes it far easier to understand what the actual function is doing,
> > avoids future mistakes in use of the confusing vma_merge() function and
> > importantly allows us to make future changes to how vma_merge() is
> > implemented by knowing explicitly which merge cases each invocation uses.
> >
> > Note that in the mremap() extend case, we perform this merge only when
> > old_len == vma->vm_end - addr. The extension_start, i.e. the start of the
> > extended portion of the VMA is equal to addr + old_len, i.e. vma->vm_end.
> >
> > With this refactoring, vma_merge() is no longer required anywhere except
> > mm/mmap.c, so mark it static.
> >
> > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

>
> Nit:
> > @@ -2546,6 +2546,24 @@ static struct vm_area_struct *vma_merge_new_vma(struct vma_iterator *vmi,
> >  			 vma->vm_userfaultfd_ctx, anon_vma_name(vma));
> >  }
> >
> > +/*
> > + * Expand vma by delta bytes, potentially merging with an immediately adjacent
> > + * VMA with identical properties.
> > + */
> > +struct vm_area_struct *vma_merge_extend(struct vma_iterator *vmi,
> > +					struct vm_area_struct *vma,
> > +					unsigned long delta)
> > +{
> > +	pgoff_t pgoff = vma->vm_pgoff +
> > +		((vma->vm_end - vma->vm_start) >> PAGE_SHIFT);
>
> could use vma_pages() here

Will update in v2.

>
> > +
> > +	/* vma is specified as prev, so case 1 or 2 will apply. */
> > +	return vma_merge(vmi, vma->vm_mm, vma, vma->vm_end, vma->vm_end + delta,
> > +			 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