Re: [RFC PATCH 1/5] mm: madvise: refactor madvise_populate()

[David Hildenbrand <david@redhat.com>]

On 19.05.25 22:52, Lorenzo Stoakes wrote:
> Use a for-loop rather than a while with the update of the start argument at
> the end of the while-loop.
> 
> This is in preparation for a subsequent commit which modifies this
> function, we therefore separate the refactoring from the actual change
> cleanly by separating the two.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
>   mm/madvise.c | 39 ++++++++++++++++++++-------------------
>   1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 8433ac9b27e0..63cc69daa4c7 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -967,32 +967,33 @@ static long madvise_populate(struct mm_struct *mm, unsigned long start,
>   	int locked = 1;
>   	long pages;
>   
> -	while (start < end) {
> +	for (; start < end; start += pages * PAGE_SIZE) {
>   		/* Populate (prefault) page tables readable/writable. */
>   		pages = faultin_page_range(mm, start, end, write, &locked);
>   		if (!locked) {
>   			mmap_read_lock(mm);
>   			locked = 1;
>   		}
> -		if (pages < 0) {
> -			switch (pages) {
> -			case -EINTR:
> -				return -EINTR;
> -			case -EINVAL: /* Incompatible mappings / permissions. */
> -				return -EINVAL;
> -			case -EHWPOISON:
> -				return -EHWPOISON;
> -			case -EFAULT: /* VM_FAULT_SIGBUS or VM_FAULT_SIGSEGV */
> -				return -EFAULT;
> -			default:
> -				pr_warn_once("%s: unhandled return value: %ld\n",
> -					     __func__, pages);
> -				fallthrough;
> -			case -ENOMEM: /* No VMA or out of memory. */
> -				return -ENOMEM;
> -			}
> +
> +		if (pages >= 0)
> +			continue;
> +
> +		switch (pages) {
> +		case -EINTR:
> +			return -EINTR;
> +		case -EINVAL: /* Incompatible mappings / permissions. */
> +			return -EINVAL;
> +		case -EHWPOISON:
> +			return -EHWPOISON;
> +		case -EFAULT: /* VM_FAULT_SIGBUS or VM_FAULT_SIGSEGV */
> +			return -EFAULT;
> +		default:
> +			pr_warn_once("%s: unhandled return value: %ld\n",
> +				     __func__, pages);
> +			fallthrough;
> +		case -ENOMEM: /* No VMA or out of memory. */
> +			return -ENOMEM;

Can we limit it to what the patch description says? "Use a for-loop 
rather than a while", or will that be a problem for the follow-up patch?

-- 
Cheers,

David / dhildenb