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

[Vlastimil Babka]

On 10/14/25 21:28, Joshua Hahn wrote:
> On Tue, 14 Oct 2025 07:50:10 -0700 Joshua Hahn <joshua.hahnjy@xxxxxxxxx> 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@xxxxxxxxx>

For the patch with this fixup:

Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>

Thanks!
> 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);