Re: [PATCH v3 21/26] mm/page_alloc: implement FREETYPE_UNMAPPED allocations

[Yosry Ahmed]

[..]
>  static __always_inline
>  struct page *rmqueue_buddy(struct zone *preferred_zone, struct zone *zone,
>  			   unsigned int order, unsigned int alloc_flags,
> @@ -3433,13 +3580,15 @@ struct page *rmqueue_buddy(struct zone *preferred_zone, struct zone *zone,
>  			 */
>  			if (!page && (alloc_flags & (ALLOC_OOM|ALLOC_HARDER)))
>  				page = __rmqueue_smallest(zone, order, ft_high);
> -
> -			if (!page) {
> -				spin_unlock_irqrestore(&zone->lock, flags);
> -				return NULL;
> -			}
>  		}
>  		spin_unlock_irqrestore(&zone->lock, flags);
> +
> +		/* Try changing direct map, now we've released the zone lock */
> +		if (!page)
> +			page = __rmqueue_direct_map(zone, order, alloc_flags, freetype);

Is it intentional that this is called outside __rmqueue() and doesn't
cover pcplists refills through rmqueue_bulk()?

IIUC, we will never change a pageblock to unmapped to refill the
pcplists, so the unmapped pcplists can get filled in two ways:
(a) When unmapped pages are freed.
(b) When a pageblock is converted here (in rmqueue_buddy()), if the
    allocation only consumes part of it, the new allocation might move
    the rest into the pcplist through rmqueue_bulk().

Does this mean that unmapped pcplists are less effective in serving
allocations? There is a tradeoff here because converting a pageblock to
unmapped is expensive, so maybe this is the right choice to make, I am
just wondering if this was intentional and/or if we tried it a different
way.

Actuall, THPs are not covered by scenario (b) above if the pageblock
size is the same as THP size, as the converted THPs are always consumed
by the allocation, so the THP pcplist will only be filled when THPs are
freed.

I wonder if this would cause a problem for THP-heavy workloads (e.g.
guest_memfd using THP, or any THP usage with ASI).

The other thing (that I probably mentioned elsewhere) is that kcompactd
does not produce unmapped pageblocks, so it seems like THP allocations
will mostly hit this code path and convert a pageblock to unmapped.
Actually, if we do bulk conversion to unmapped (e.g. in kcompactd) we
could batch the TLB shootdowns as well, but that should probably be done
separately.


> +		if (!page)
> +			return NULL;
> +
>  	} while (check_new_pages(page, order));
>  
>  	/*