Re: [PATCH 4/7] mm/mremap: initial refactor of move_vma()

[Yosry Ahmed]

On Mon, Mar 03, 2025 at 11:08:34AM +0000, Lorenzo Stoakes wrote:
> Update move_vma() to use the threaded VRM object, de-duplicate code and
> separate into smaller functions to aid readability and debug-ability.
> 
> This in turn allows further simplification of expand_vma() as we can simply
> thread VRM through the function.
> 
> We also take the opportunity to abstract the account charging page count
> into the VRM in order that we can correctly thread this through the
> operation.
> 
> We additionally do the same for tracking mm statistics - exec_vm, stack_vm,
> data_vm, and locked_vm.
> 
> As part of this change, we slightly modify when locked pages statistics are
> counted for in mm_struct statistics. However this should cause no issues,
> as there is no chance of underflow, nor will any rlimit failures occur as a
> result.
> 
> This is an intermediate step before a further refactoring of move_vma() in
> order to aid review.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> ---
[..]
> +/*
> + * Perform checks  before attempting to write a VMA prior to it being
> + * moved.
> + */
> +static unsigned long prep_move_vma(struct vma_remap_struct *vrm,
> +				   unsigned long *vm_flags_ptr)
> +{
> +	unsigned long err;

I am getting a warning on mm-unstable because 'err' is sometimes used
uninitialized, I think here:

        if (vma->vm_ops && vma->vm_ops->may_split) {
                if (vma->vm_start != old_addr)
                        err = vma->vm_ops->may_split(vma, old_addr);
                if (!err && vma->vm_end != old_addr + old_len)
                        err = vma->vm_ops->may_split(vma, old_addr + old_len);
                if (err)
                        return err;
        }

> +	struct vm_area_struct *vma = vrm->vma;
> +	unsigned long old_addr = vrm->addr;
> +	unsigned long old_len = vrm->old_len;
>  
>  	/*
>  	 * We'd prefer to avoid failure later on in do_munmap:
>  	 * which may split one vma into three before unmapping.
>  	 */
> -	if (mm->map_count >= sysctl_max_map_count - 3)
> +	if (current->mm->map_count >= sysctl_max_map_count - 3)
>  		return -ENOMEM;
>  
> -	if (unlikely(flags & MREMAP_DONTUNMAP))
> -		to_account = new_len;
> -
>  	if (vma->vm_ops && vma->vm_ops->may_split) {
>  		if (vma->vm_start != old_addr)
>  			err = vma->vm_ops->may_split(vma, old_addr);
[..]