linux-kernel - Re: [PATCH v5 3/3] mm/page_alloc: Batch page freeing in free_frozen_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20564b23-a3eb-4282-acec-be8913a84daa@suse.cz>
Date: Tue, 14 Oct 2025 22:45:06 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Joshua Hahn <joshua.hahnjy@...il.com>,
 Andrew Morton <akpm@...ux-foundation.org>
Cc: Chris Mason <clm@...com>, Kiryl Shutsemau <kirill@...temov.name>,
 Brendan Jackman <jackmanb@...gle.com>, Johannes Weiner <hannes@...xchg.org>,
 Michal Hocko <mhocko@...e.com>, Suren Baghdasaryan <surenb@...gle.com>,
 Zi Yan <ziy@...dia.com>, linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 kernel-team@...a.com
Subject: Re: [PATCH v5 3/3] mm/page_alloc: Batch page freeing in
 free_frozen_page_commit

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

For the patch with this fixup:

Reviewed-by: Vlastimil Babka <vbabka@...e.cz>

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