Re: [PATCH v3 1/2] mm/memory: simplify error handling in insert_pages()

[Lorenzo Stoakes (ARM)]

On Sat, Aug 29, 2026 at 08:11:12PM +0300, Avi Weiss wrote:
> Initialize error return status to zero and then set it as needed at each
> point of failure.
>
> Assign -ENOMEM explicitly when pte_alloc() fails as the pte_alloc()
> macro returns a boolean.
>
> Signed-off-by: Avi Weiss <thnkslprpt@xxxxxxxxx>
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>

> ---
>  mm/memory.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 9cbce5c90bff..2561dc6bdde6 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2565,20 +2565,22 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
>  	unsigned long curr_page_idx = 0;
>  	unsigned long remaining_pages_total = *num;
>  	unsigned long pages_to_write_in_pmd;
> -	int ret;
> +	int err = 0;
>  more:
> -	ret = -EFAULT;
>  	pmd = walk_to_pmd(mm, addr);
> -	if (!pmd)
> +	if (!pmd) {
> +		err = -EFAULT;
>  		goto out;
> +	}
>
>  	pages_to_write_in_pmd = min_t(unsigned long,
>  		remaining_pages_total, PTRS_PER_PTE - pte_index(addr));
>
>  	/* Allocate the PTE if necessary; takes PMD lock once only. */
> -	ret = -ENOMEM;
> -	if (pte_alloc(mm, pmd))
> +	if (pte_alloc(mm, pmd)) {
> +		err = -ENOMEM;
>  		goto out;
> +	}
>
>  	while (pages_to_write_in_pmd) {
>  		int pte_idx = 0;
> @@ -2586,15 +2588,14 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
>
>  		start_pte = pte_offset_map_lock(mm, pmd, addr, &pte_lock);
>  		if (!start_pte) {
> -			ret = -EFAULT;
> +			err = -EFAULT;
>  			goto out;
>  		}
>  		for (pte = start_pte; pte_idx < batch_size; ++pte, ++pte_idx) {
> -			int err = insert_page_in_batch_locked(vma, pte,
> -				addr, pages[curr_page_idx], prot);
> +			err = insert_page_in_batch_locked(vma, pte, addr,
> +							  pages[curr_page_idx], prot);
>  			if (unlikely(err)) {
>  				pte_unmap_unlock(start_pte, pte_lock);
> -				ret = err;
>  				remaining_pages_total -= pte_idx;
>  				goto out;
>  			}
> @@ -2607,10 +2608,9 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
>  	}
>  	if (remaining_pages_total)
>  		goto more;
> -	ret = 0;
>  out:
>  	*num = remaining_pages_total;
> -	return ret;
> +	return err;
>  }
>
>  /**
> --
> 2.43.0
>

--
Cheers, Lorenzo