On 9/11/23 21:41, Johannes Weiner wrote:
> Currently, page block type conversion during fallbacks, atomic
> reservations and isolation can strand various amounts of free pages on
> incorrect freelists.
>
> For example, fallback stealing moves free pages in the block to the
> new type's freelists, but then may not actually claim the block for
> that type if there aren't enough compatible pages already allocated.
>
> In all cases, free page moving might fail if the block straddles more
> than one zone, in which case no free pages are moved at all, but the
> block type is changed anyway.
>
> This is detrimental to type hygiene on the freelists. It encourages
> incompatible page mixing down the line (ask for one type, get another)
> and thus contributes to long-term fragmentation.
>
> Split the process into a proper transaction: check first if conversion
> will happen, then try to move the free pages, and only if that was
> successful convert the block to the new type.
>
> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
<snip>
> @@ -1638,26 +1629,62 @@ static int move_freepages(struct zone *zone,
> return pages_moved;
> }
>
> -int move_freepages_block(struct zone *zone, struct page *page,
> - int migratetype, int *num_movable)
> +static bool prep_move_freepages_block(struct zone *zone, struct page *page,
> + unsigned long *start_pfn,
> + unsigned long *end_pfn,
> + int *num_free, int *num_movable)
> {
> - unsigned long start_pfn, end_pfn, pfn;
> -
> - if (num_movable)
> - *num_movable = 0;
> + unsigned long pfn, start, end;
>
> pfn = page_to_pfn(page);
> - start_pfn = pageblock_start_pfn(pfn);
> - end_pfn = pageblock_end_pfn(pfn) - 1;
> + start = pageblock_start_pfn(pfn);
> + end = pageblock_end_pfn(pfn) - 1;
> /* Do not cross zone boundaries */
> - if (!zone_spans_pfn(zone, start_pfn))
> - start_pfn = zone->zone_start_pfn;
> - if (!zone_spans_pfn(zone, end_pfn))
> - return 0;
> + if (!zone_spans_pfn(zone, start))
> + start = zone->zone_start_pfn;
> + if (!zone_spans_pfn(zone, end))
> + return false;
This brings me back to my previous suggestion - if we update the end, won't
the whole "block straddles >1 zones" situation to check for go away?
Hm or is it actually done because we have a problem by representing
pageblock migratetype with multiple zones, since there's a single
pageblock_bitmap entry per the respective pageblock range of pfn's, so one
zone's migratetype could mess with other's? And now it matters if we want
100% match of freelist vs pageblock migratetype?
(I think even before this series it could have mattered for
MIGRATETYPE_ISOLATE, is it broken in those corner cases?)
But in that case we might not be detecting the situation properly for the
later of the two zones in a pageblock, because if start_pfn is not spanned
we adjust it and continue? Hmm...
