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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251014192827.851389-1-joshua.hahnjy@gmail.com>
Date: Tue, 14 Oct 2025 12:28:22 -0700
From: Joshua Hahn <joshua.hahnjy@...il.com>
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>,
	Vlastimil Babka <vbabka@...e.cz>,
	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 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>

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