On Tue, May 12, 2026 at 1:30 AM David Hildenbrand (Arm)
<[email protected]> wrote:
>
> 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 <[email protected]>
> > Acked-by: David Hildenbrand (Arm) <[email protected]>
>
> Some nits when re-reading:
>
> > Acked-by: Usama Arif <[email protected]>
> > Signed-off-by: Nico Pache <[email protected]>
> > ---
> > 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".
Yeah I think you mentioned that last review... sorry!
>
> 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.
Hmm tbh I couldn't find a example of what you meant here. There are
some that put a space between the first sentence and the @ list.
>
> > + * @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.
Dropped it, thanks.
>
> > + 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
>
