Re: [PATCH v2 4/5] mm/mremap: restore VMA_ACCOUNT_BIT if the unmap fails

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Mon, Nov 02, 2026 at 11:46:00PM +0000, Josef Bacik wrote:
> If the unmap at the end of an mremap() move fails, the source VMA stays
> without VMA_ACCOUNT_BIT. Neither munmap() nor exit_mmap() uncharges it
> then, so the charge for all of its pages stays in vm_committed_as after
> the process is gone.
>
> unmap_source_vma() clears the flag on the whole source VMA before the unmap
> and sets it again on the remaining parts afterwards, but its error path
> returns without doing so.
>
> This was reproduced with fault injection: each failure on an anonymous
> mapping left Committed_AS higher by the size of the whole VMA after
> everything was unmapped.
>
> Fix by setting the flag again on everything in the range of the original
> VMA, on both paths, with a new helper. A failed unmap may have split the
> VMA already, so the helper walks the range.
>
> Fixes: 51df7bcb6151 ("mm/mremap: account memory on do_munmap() failure")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Thanks, LGTM!

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  mm/mremap.c | 41 +++++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 22 deletions(-)

Nice diffstat :)

>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 71c9f335bea8..4d1e446620e2 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1189,6 +1189,19 @@ static unsigned long prep_move_vma(struct vma_remap_struct *vrm)
>  	return 0;
>  }
>
> +/* Set VMA_ACCOUNT_BIT on all VMAs in the range [start, end). */
> +static void vma_range_set_account(struct vma_iterator *vmi,
> +				  unsigned long start, unsigned long end)
> +{
> +	struct vm_area_struct *vma;
> +
> +	vma_iter_set(vmi, start);
> +	for_each_vma_range(*vmi, vma, end) {
> +		vma_start_write(vma);
> +		vma_set_flags(vma, VMA_ACCOUNT_BIT);
> +	}
> +}
> +
>  /*
>   * Unmap source VMA for VMA move, turning it from a copy to a move, being
>   * careful to ensure we do not underflow memory account while doing so if an
> @@ -1246,8 +1259,10 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)
>  	vrm->vmi_needs_invalidate = true;
>  	if (err) {
>  		/* OOM: unable to split vma, just get accounts right */
> -		if (accountable_move)
> +		if (accountable_move) {
>  			vm_acct_memory(len >> PAGE_SHIFT);
> +			vma_range_set_account(&vmi, vm_start, vm_end);
> +		}
>  		return;
>  	}
>
> @@ -1271,28 +1286,10 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)
>  	 * | A |     | B |
>  	 * |---|     |---|
>  	 *
> -	 * The VMI is still pointing at addr, so vma_prev() will give us A, and
> -	 * a subsequent or lone vma_next() will give as B.
> -	 *
> -	 * do_vmi_munmap() will have restored the VMI back to addr.
> +	 * So reinstate the flag on whatever is left of the range.
>  	 */
> -	if (accountable_move) {
> -		unsigned long end = addr + len;
> -
> -		if (vm_start < addr) {
> -			struct vm_area_struct *prev = vma_prev(&vmi);
> -
> -			vma_start_write(prev);
> -			vma_set_flags(prev, VMA_ACCOUNT_BIT);
> -		}
> -
> -		if (vm_end > end) {
> -			struct vm_area_struct *next = vma_next(&vmi);
> -
> -			vma_start_write(next);
> -			vma_set_flags(next, VMA_ACCOUNT_BIT);
> -		}
> -	}
> +	if (accountable_move)
> +		vma_range_set_account(&vmi, vm_start, vm_end);
>  }
>
>  /*
>
> --
> 2.55.0
>

--
Cheers, Lorenzo