Re: [PATCH 05/10] mm/madvise: factor huge-PMD folio processing

[Gregory Price <gourry@gourry.net>]

On Wed, Sep 23, 2026 at 05:43:59PM +0100, Lorenzo Stoakes (ARM) wrote:
> > +/* Return a locked, referenced folio only when it must be split. */
> 
> I find it really weird that when it:
> 
> a. succeeds
> b. mapped folio is missing/invalid/filtered
> 
> In both cases it returns NULL.
> 
> And it's also weirdly returning a folio in a kind of failure case, or it's
> more like a defer-to-the-rest-of-the-code case I suppose.
> 
> I wonder if the split could be done as part of the function?
> 
> Then maybe have it return bool and document that true means it's fully
> processed (invalid folio cases, success case), false means that it's been
> split and the rest of the code should continue.
> 
> Awkward one actually.

Yes this was an awkward one to futz around with.  I took a couple tries
at it and this is ultimately what fell out and passed the tests.

I think there's some tweaks that could be made here, but I err'd on the
side of "don't break shit" before I went twiddling.

It is at least easier to understand, but certainly this shows how poorly
the original code was structured.

> 
> > +static struct folio *
> > +madvise_lru_huge_pmd_locked(pmd_t *pmd, pmd_t orig_pmd,
> > +		unsigned long addr, unsigned long next, struct mm_walk *walk,
> > +		struct list_head *folio_list, bool pageout_anon_only)
> > +{
> > +	const struct madvise_walk_private *private = walk->private;
> > +	struct vm_area_struct *vma = walk->vma;
> > +	struct folio *folio;
> > +
> > +	folio = vm_normal_folio_pmd(vma, addr, orig_pmd);
> > +	if (!folio || folio_is_zone_device(folio))
> > +		return NULL;
> > +	if (madvise_lru_folio_is_filtered(folio, pageout_anon_only))
> > +		return NULL;
> > +
> > +	if (next - addr != HPAGE_PMD_SIZE) {
> 
> NIT: Maybe could define above as:
> 
> 	const bool spans_pmd = next - addr == HPAGE_PMD_SIZE;
> 
> And then make this:
> 
> 	if (!spans_pmd)
> 

seems reasonable. ack

~Gregory