Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <fe147a1e-6fe5-309f-b2e7-48f5f3c97bae@suse.cz>
Date:   Mon, 9 Oct 2023 18:30:02 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH 4/4] mm: abstract VMA extension and merge into
 vma_merge_extend() helper
Content-Language: en-US
To:     Lorenzo Stoakes <lstoakes@gmail.com>, 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>
Cc:     "=Liam R . Howlett" <Liam.Howlett@oracle.com>,
        linux-fsdevel@vger.kernel.org
References: <cover.1696795837.git.lstoakes@gmail.com>
 <1ed3d1ba0069104e1685298aa2baf980c38a85ff.1696795837.git.lstoakes@gmail.com>
From:   Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <1ed3d1ba0069104e1685298aa2baf980c38a85ff.1696795837.git.lstoakes@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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>

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

> +
> +	/* 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