Re: [RFC PATCH 33/40] PM: hibernate: walk per-superpageblock free lists in mark_free_pages

[Rafael J. Wysocki]

On Wed, May 20, 2026 at 5:01 PM Rik van Riel <riel@xxxxxxxxxxx> wrote:
>
> mark_free_pages() walks the buddy allocator's free lists and calls
> swsusp_set_page_free() on each free page so it is omitted from the
> hibernation image. After the SPB rework, free pages live on per-
> superpageblock free lists rather than the zone-level free list, so
> the existing list_for_each_entry() walk over zone->free_area[order]
> .free_list[t] found nothing. The hibernation snapshot then treated
> every free page as needing to be saved, wasting image space and risking
> OOM during the snapshot.
>
> Wrap the existing per-page walk in an SPB iteration loop. When the
> zone has no SPBs (e.g. an unpopulated hotplug zone), fall back to the
> zone-level free list. The whole function still runs under
> spin_lock_irqsave(&zone->lock) without drops, so there are no lock-
> order or hotplug concerns.
>
> Build-tested as part of the full mm series; kernel/power/snapshot.o
> compiles cleanly in that build.  An -Werror=return-type warning in
> enough_free_mem() exists on some configurations but is unrelated to
> this change and predates the SPB series.
>
> Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx>
> Cc: Len Brown <lenb@xxxxxxxxxx>
> Cc: Pavel Machek <pavel@xxxxxxxxxx>
> Cc: linux-pm@xxxxxxxxxxxxxxx
> Signed-off-by: Rik van Riel <riel@xxxxxxxxxxx>
> Assisted-by: Claude:claude-opus-4.7 syzkaller
> ---
>  kernel/power/snapshot.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index a564650734dc..f96885bc46f8 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1270,17 +1270,32 @@ static void mark_free_pages(struct zone *zone)
>         }
>
>         for_each_migratetype_order(order, t) {
> -               list_for_each_entry(page,
> -                               &zone->free_area[order].free_list[t], buddy_list) {
> -                       unsigned long i;
> -
> -                       pfn = page_to_pfn(page);
> -                       for (i = 0; i < (1UL << order); i++) {
> -                               if (!--page_count) {
> -                                       touch_nmi_watchdog();
> -                                       page_count = WD_PAGE_COUNT;
> +               unsigned long sb_idx;
> +               unsigned long nr_lists = zone->nr_superpageblocks ? : 1;
> +
> +               /*
> +                * After the SPB rework, free pages live on per-superpageblock
> +                * free lists. Walk every SPB's list for this (order, mt) cell.
> +                * If the zone has no SPBs (unpopulated zone), fall back to the
> +                * zone-level list head so that any pre-SPB pages are still
> +                * marked.
> +                */
> +               for (sb_idx = 0; sb_idx < nr_lists; sb_idx++) {
> +                       struct list_head *list = zone->nr_superpageblocks ?
> +                               &zone->superpageblocks[sb_idx].free_area[order].free_list[t] :
> +                               &zone->free_area[order].free_list[t];
> +

Suppose the code below goes into a separate function returning the
current page_count value and taking list, buddy_list and page_cout as
arguments.  Say its name is do_mark_free_pages().

> +                       list_for_each_entry(page, list, buddy_list) {
> +                               unsigned long i;
> +
> +                               pfn = page_to_pfn(page);
> +                               for (i = 0; i < (1UL << order); i++) {
> +                                       if (!--page_count) {
> +                                               touch_nmi_watchdog();
> +                                               page_count = WD_PAGE_COUNT;
> +                                       }
> +                                       swsusp_set_page_free(pfn_to_page(pfn + i));
>                                 }
> -                               swsusp_set_page_free(pfn_to_page(pfn + i));
>                         }
>                 }
>         }
> --

Then, what you seem to be wanting to do is roughly

if (zone->nr_superpageblocks == 0) {
        do_mark_free_pages(&zone->free_area[order].free_list[t],
buddy_list, page_count);
} else {
        for (sb_idx = 0; sb_idx < zone->nr_superpageblocks; sb_idx++) {
                page_count =
do_mark_free_pages(&zone->superpageblocks[sb_idx].free_area[order].free_list[t],

   buddy_list, page_count);
        }
}

which would be kind of easier to follow.