Re: [PATCH v5 3/3] mm/page_alloc: Batch page freeing in free_frozen_page_commit

[Joshua Hahn <joshua.hahnjy@gmail.com>]

On Tue, 14 Oct 2025 07:50:10 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> Before returning, free_frozen_page_commit calls free_pcppages_bulk using
> nr_pcp_free to determine how many pages can appropritately be freed,
> based on the tunable parameters stored in pcp. While this number is an
> accurate representation of how many pages should be freed in total, it
> is not an appropriate number of pages to free at once using
> free_pcppages_bulk, since we have seen the value consistently go above
> 2000 in the Meta fleet on larger machines.
> 
> As such, perform batched page freeing in free_pcppages_bulk by using
> pcp->batch. In order to ensure that other processes are not starved of the
> zone lock, free both the zone lock and pcp lock to yield to other threads.
> 
> Note that because free_frozen_page_commit now performs a spinlock inside the
> function (and can fail), the function may now return with a freed pcp.
> To handle this, return true if the pcp is locked on exit and false otherwise.
> 
> In addition, since free_frozen_page_commit must now be aware of what UP
> flags were stored at the time of the spin lock, and because we must be
> able to report new UP flags to the callers, add a new unsigned long*
> parameter UP_flags to keep track of this.

[...snip...]

Hello Andrew, I hope you are doing well! I was wondering if you could help
adding this as a fixlet for the patch I am writing this reply to. Vlastimil
kindly pointed out that they should never go negative, so checking for
0-ness should be sufficient and more readable than the <= checks.

I think it is OK to leave the changelog in 0/3 unchanged, since it will not go
into the commit history and Vlastimil has already left a correction. But
please let me know if you would like me to add a correction for that as well.

Thank you as always, for your help! I hope you have a great day!
Joshua

...

Since to_free and pcp->count cannot become negative, make the checks into an
equality check instead.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6d544521e49c..fd46a982ce3c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2884,7 +2884,7 @@ static bool free_frozen_page_commit(struct zone *zone,
                free_pcppages_bulk(zone, to_free_batched, pcp, pindex);
                to_free -= to_free_batched;

-               if (to_free <= 0 || pcp->count <= 0)
+               if (to_free == 0 || pcp->count == 0)
                        break;

                pcp_spin_unlock(pcp);