Date:   Wed, 15 Mar 2023 21:38:37 +0000
From:   Lorenzo Stoakes <lstoakes@gmail.com>
To:     Vlastimil Babka <vbabka@suse.cz>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        "Liam R. Howlett" <Liam.Howlett@oracle.com>,
        Matthew Wilcox <willy@infradead.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        maple-tree@lists.infradead.org
Subject: Re: [PATCH 07/10] mm/mmap/vma_merge: rename adj_next to adj_start
Message-ID: <f6a24126-99b8-4b6b-a456-bd6633ec491b@lucifer.local>
References: <20230309111258.24079-1-vbabka@suse.cz>
 <20230309111258.24079-8-vbabka@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230309111258.24079-8-vbabka@suse.cz>
Precedence: bulk

On Thu, Mar 09, 2023 at 12:12:55PM +0100, Vlastimil Babka wrote:
> The variable 'adj_next' holds the value by which we adjust vm_start of a
> vma in variable 'adjust', that's either 'next' or 'mid', so the current
> name is inaccurate. Rename it to 'adj_start'.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  mm/mmap.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 3396c9b13f1c..c51d69592e4e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -903,7 +903,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  	bool vma_expanded = false;
>  	struct vma_prepare vp;
>  	unsigned long vma_end = end;
> -	long adj_next = 0;
> +	long adj_start = 0;
>  	unsigned long vma_start = addr;
>
>  	validate_mm(mm);
> @@ -971,7 +971,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  				remove = mid;
>  			} else {			/* case 5 */
>  				adjust = mid;
> -				adj_next = (end - mid->vm_start);
> +				adj_start = (end - mid->vm_start);
>  			}
>  		}
>  	} else if (merge_next) {
> @@ -979,7 +979,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  		if (prev && addr < prev->vm_end) {	/* case 4 */
>  			vma_end = addr;
>  			adjust = next;
> -			adj_next = -(prev->vm_end - addr);
> +			adj_start = -(prev->vm_end - addr);
>  			err = dup_anon_vma(next, prev);
>  		} else {
>  			vma = next;			/* case 3 */
> @@ -1002,7 +1002,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  	if (vma_iter_prealloc(vmi))
>  		return NULL;
>
> -	vma_adjust_trans_huge(vma, vma_start, vma_end, adj_next);
> +	vma_adjust_trans_huge(vma, vma_start, vma_end, adj_start);
>  	init_multi_vma_prep(&vp, vma, adjust, remove, remove2);
>  	VM_WARN_ON(vp.anon_vma && adjust && adjust->anon_vma &&
>  		   vp.anon_vma != adjust->anon_vma);
> @@ -1018,10 +1018,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  	if (vma_expanded)
>  		vma_iter_store(vmi, vma);
>
> -	if (adj_next) {
> -		adjust->vm_start += adj_next;
> -		adjust->vm_pgoff += adj_next >> PAGE_SHIFT;
> -		if (adj_next < 0) {
> +	if (adj_start) {
> +		adjust->vm_start += adj_start;
> +		adjust->vm_pgoff += adj_start >> PAGE_SHIFT;
> +		if (adj_start < 0) {
>  			WARN_ON(vma_expanded);
>  			vma_iter_store(vmi, next);
>  		}
> --
> 2.39.2
>

This feels like a very silly oversight in the first instance but I imagine one
that is a result of 'code evolution' :) excellent improvement in clarity, so:-

Reviewed-By: Lorenzo Stoakes <lstoakes@gmail.com>