Re: [PATCH mm-unstable v17 03/14] mm/khugepaged: rework max_ptes_* handling with helper functions

[David Hildenbrand (Arm)]

On 5/11/26 20:58, Nico Pache wrote:
> The following cleanup reworks all the max_ptes_* handling into helper
> functions. This increases the code readability and will later be used to
> implement the mTHP handling of these variables.
> 
> With these changes we abstract all the madvise_collapse() special casing
> (dont respect the sysctls) away from the functions that utilize them. And
> will be used later in this series to cleanly restrict the mTHP collapse
> behavior.
> 
> No functional change is intended; however, we are now only reading the
> sysfs variables once per scan, whereas before these variables were being
> read on each loop iteration.
> 
> Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Some nits when re-reading:

> Acked-by: Usama Arif <usama.arif@xxxxxxxxx>
> Signed-off-by: Nico Pache <npache@xxxxxxxxxx>
> ---
>  mm/khugepaged.c | 118 +++++++++++++++++++++++++++++++++---------------
>  1 file changed, 82 insertions(+), 36 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index f0e29d5c7b1f..f68853b3caa7 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -348,6 +348,62 @@ static bool pte_none_or_zero(pte_t pte)
>  	return pte_present(pte) && is_zero_pfn(pte_pfn(pte));
>  }
>  
> +/**
> + * collapse_max_ptes_none - Calculate maximum allowed none-page or zero-page

I know, it's painful, but ...

There is no "none-page".

Calculate maximum allowed empty PTEs or PTEs mapping the shared zeropage ... ?

> + * PTEs for the given collapse operation.

We usually indent here (second line of subject), I think. Same applies to the
other doc below.

> + * @cc: The collapse control struct
> + * @vma: The vma to check for userfaultfd
> + *
> + * Return: Maximum number of none-page or zero-page PTEs allowed for the
> + * collapse operation.

Same here.

> + */
> +static unsigned int collapse_max_ptes_none(struct collapse_control *cc,
> +		struct vm_area_struct *vma)
> +{
> +	// If the vma is userfaultfd-armed, allow no none-page or zero-page PTEs.

Lance commented on the comment style.

Is this comment really required? It's pretty self-documenting already.

> +	if (vma && userfaultfd_armed(vma))
> +		return 0;
> +	// for MADV_COLLAPSE, allow any none-page or zero-page PTEs.
> +	if (!cc->is_khugepaged)
> +		return HPAGE_PMD_NR;
> +	// For all other cases repect the user defined maximum.
> +	return khugepaged_max_ptes_none;
> +}
> +
-- 
Cheers,

David