[PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[Yajun Deng]

In move_to_free_list(), when a page block changes its migration type,
we need to update free page counts for both the old and new types.
Originally, this was done by two calls to account_freepages(), which
updates NR_FREE_PAGES and also type-specific counters. However, this
causes NR_FREE_PAGES to be updated twice, while the net change is zero
in most cases.

This patch introduces a new function account_freepages_both() that
updates the statistics for both old and new migration types in one go.
It avoids the double update of NR_FREE_PAGES by computing the net change
only when the isolation status changes.

The optimization avoid duplicate NR_FREE_PAGES updates in
move_to_free_list().

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 mm/page_alloc.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ebfa07632995..e51d8bd7ab7d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -812,6 +812,16 @@ compaction_capture(struct capture_control *capc, struct page *page,
 }
 #endif /* CONFIG_COMPACTION */
 
+static inline void account_specific_freepages(struct zone *zone, int nr_pages,
+					      int migratetype)
+{
+	if (is_migrate_cma(migratetype))
+		__mod_zone_page_state(zone, NR_FREE_CMA_PAGES, nr_pages);
+	else if (migratetype == MIGRATE_HIGHATOMIC)
+		WRITE_ONCE(zone->nr_free_highatomic,
+			   zone->nr_free_highatomic + nr_pages);
+}
+
 static inline void account_freepages(struct zone *zone, int nr_pages,
 				     int migratetype)
 {
@@ -822,11 +832,25 @@ static inline void account_freepages(struct zone *zone, int nr_pages,
 
 	__mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
 
-	if (is_migrate_cma(migratetype))
-		__mod_zone_page_state(zone, NR_FREE_CMA_PAGES, nr_pages);
-	else if (migratetype == MIGRATE_HIGHATOMIC)
-		WRITE_ONCE(zone->nr_free_highatomic,
-			   zone->nr_free_highatomic + nr_pages);
+	account_specific_freepages(zone, nr_pages, migratetype);
+}
+
+static inline void account_freepages_both(struct zone *zone, int nr_pages,
+					  int old_mt, int new_mt)
+{
+	lockdep_assert_held(&zone->lock);
+
+	bool old_isolated = is_migrate_isolate(old_mt);
+	bool new_isolated = is_migrate_isolate(new_mt);
+
+	if (old_isolated != new_isolated)
+		__mod_zone_page_state(zone, NR_FREE_PAGES,
+				      old_isolated ? nr_pages : -nr_pages);
+
+	if (!old_isolated)
+		account_specific_freepages(zone, -nr_pages, old_mt);
+	if (!new_isolated)
+		account_specific_freepages(zone, nr_pages, new_mt);
 }
 
 /* Used for pages not on another list */
@@ -869,8 +893,7 @@ static inline void move_to_free_list(struct page *page, struct zone *zone,
 
 	list_move_tail(&page->buddy_list, &area->free_list[new_mt]);
 
-	account_freepages(zone, -nr_pages, old_mt);
-	account_freepages(zone, nr_pages, new_mt);
+	account_freepages_both(zone, nr_pages, old_mt, new_mt);
 
 	if (order >= pageblock_order &&
 	    is_migrate_isolate(old_mt) != is_migrate_isolate(new_mt)) {
-- 
2.34.1

Re: [PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[Joshua Hahn]

On Fri,  9 Jan 2026 18:51:21 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:

> In move_to_free_list(), when a page block changes its migration type,
> we need to update free page counts for both the old and new types.
> Originally, this was done by two calls to account_freepages(), which
> updates NR_FREE_PAGES and also type-specific counters. However, this
> causes NR_FREE_PAGES to be updated twice, while the net change is zero
> in most cases.
> 
> This patch introduces a new function account_freepages_both() that
> updates the statistics for both old and new migration types in one go.
> It avoids the double update of NR_FREE_PAGES by computing the net change
> only when the isolation status changes.
> 
> The optimization avoid duplicate NR_FREE_PAGES updates in
> move_to_free_list().

Hi Yajun,

I hope you are doing well, thank you for the patch! I was hoping to better
understand the motivation behind this patch.

From my perspective, I believe that the current state of the code is
not optimal, but it is also not problematic. account_freepages seems like
a relatively cheap function (at the core, it's just some atomic operations).
Personally I also think that semantically, the code currently makes sense;
we are doing the accounting for the old mounttype, then for the new mounttype,
in a way that cancels out. And given that there is still some cases where
the work doesn't end up canceling out due to one of the mounttypes being
MIGRATE_ISOLATE, I think that there is enough purpose in making the two
calls to do the accounting twice.

On the other hand I think there is only one place in the codebase that
will use account_freepages_both, so it might make the burden to understand
the code a bit higher.

What do you think? I don't have a strong stance on whether the performance
effects are big here (if this change indeed has a big performance implication,
then we should definitely go forth with this!) but I do believe the current
code is quite semantically sound and more readable. 

Thank you again for the patch. I hope you have a great day!
Joshua

Re: [PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[Yajun Deng]

> 2026年1月10日 00:31，Joshua Hahn <joshua.hahnjy@gmail.com> 写道：
> 
> On Fri,  9 Jan 2026 18:51:21 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:
> 
>> In move_to_free_list(), when a page block changes its migration type,
>> we need to update free page counts for both the old and new types.
>> Originally, this was done by two calls to account_freepages(), which
>> updates NR_FREE_PAGES and also type-specific counters. However, this
>> causes NR_FREE_PAGES to be updated twice, while the net change is zero
>> in most cases.
>> 
>> This patch introduces a new function account_freepages_both() that
>> updates the statistics for both old and new migration types in one go.
>> It avoids the double update of NR_FREE_PAGES by computing the net change
>> only when the isolation status changes.
>> 
>> The optimization avoid duplicate NR_FREE_PAGES updates in
>> move_to_free_list().
> 
> Hi Yajun,
> 
> I hope you are doing well, thank you for the patch! I was hoping to better
> understand the motivation behind this patch.
> 
> From my perspective, I believe that the current state of the code is
> not optimal, but it is also not problematic. account_freepages seems like
> a relatively cheap function (at the core, it's just some atomic operations).
> Personally I also think that semantically, the code currently makes sense;
> we are doing the accounting for the old mounttype, then for the new mounttype,
> in a way that cancels out. And given that there is still some cases where
> the work doesn't end up canceling out due to one of the mounttypes being
> MIGRATE_ISOLATE, I think that there is enough purpose in making the two
> calls to do the accounting twice.
> 
> On the other hand I think there is only one place in the codebase that
> will use account_freepages_both, so it might make the burden to understand
> the code a bit higher.
> 
> What do you think? I don't have a strong stance on whether the performance
> effects are big here (if this change indeed has a big performance implication,
> then we should definitely go forth with this!) but I do believe the current
> code is quite semantically sound and more readable. 
> 
Hey Joshua,

Thank you for sharing your thoughts. 

I currently don’t have any performance data, I just noticed from looking at the code
that there may be room for optimization.
You’re right. The original code is indeed more straightforward. I think we can add some
comments in the  account_freepages_both to make it easier to understand.

> Thank you again for the patch. I hope you have a great day!
> Joshua

Re: [PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[Joshua Hahn]

On Sun, 11 Jan 2026 21:47:42 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:

> 
> 
> > 2026年1月10日 00:31，Joshua Hahn <joshua.hahnjy@gmail.com> 写道：
> > 
> > On Fri,  9 Jan 2026 18:51:21 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:
> > 
> >> In move_to_free_list(), when a page block changes its migration type,
> >> we need to update free page counts for both the old and new types.
> >> Originally, this was done by two calls to account_freepages(), which
> >> updates NR_FREE_PAGES and also type-specific counters. However, this
> >> causes NR_FREE_PAGES to be updated twice, while the net change is zero
> >> in most cases.
> >> 
> >> This patch introduces a new function account_freepages_both() that
> >> updates the statistics for both old and new migration types in one go.
> >> It avoids the double update of NR_FREE_PAGES by computing the net change
> >> only when the isolation status changes.
> >> 
> >> The optimization avoid duplicate NR_FREE_PAGES updates in
> >> move_to_free_list().
> > 
> > Hi Yajun,
> > 
> > I hope you are doing well, thank you for the patch! I was hoping to better
> > understand the motivation behind this patch.
> > 
> > From my perspective, I believe that the current state of the code is
> > not optimal, but it is also not problematic. account_freepages seems like
> > a relatively cheap function (at the core, it's just some atomic operations).
> > Personally I also think that semantically, the code currently makes sense;
> > we are doing the accounting for the old mounttype, then for the new mounttype,
> > in a way that cancels out. And given that there is still some cases where
> > the work doesn't end up canceling out due to one of the mounttypes being
> > MIGRATE_ISOLATE, I think that there is enough purpose in making the two
> > calls to do the accounting twice.
> > 
> > On the other hand I think there is only one place in the codebase that
> > will use account_freepages_both, so it might make the burden to understand
> > the code a bit higher.
> > 
> > What do you think? I don't have a strong stance on whether the performance
> > effects are big here (if this change indeed has a big performance implication,
> > then we should definitely go forth with this!) but I do believe the current
> > code is quite semantically sound and more readable. 
> > 
> Hey Joshua,
> 
> Thank you for sharing your thoughts. 
> 
> I currently don’t have any performance data, I just noticed from looking at the code
> that there may be room for optimization.
> You’re right. The original code is indeed more straightforward. I think we can add some
> comments in the  account_freepages_both to make it easier to understand.

Hi Yajun, I hope you are doing well!

On second thought, I did notice that at the end of move_to_free_list, we have
some additional conditionals that depend on the migratetype of the mounttypes.

What if we open-code the account_freepages_both, and skip doing the
isolation checks twice? Your idea to use the ternary operator gave me this idea!

@@ -869,14 +877,17 @@ static inline void move_to_free_list(struct page *page, struct zone *zone,

        list_move_tail(&page->buddy_list, &area->free_list[new_mt]);

-       account_freepages(zone, -nr_pages, old_mt);
-       account_freepages(zone, nr_pages, new_mt);
-
-       if (order >= pageblock_order &&
-           is_migrate_isolate(old_mt) != is_migrate_isolate(new_mt)) {
-               if (!is_migrate_isolate(old_mt))
-                       nr_pages = -nr_pages;
-               __mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS, nr_pages);
+       if (!old_isolated)
+               account_specific_freepages(zone, -nr_pages, old_mt);
+       if !(new_isolated)
+               account_specific_freepages(zone, nr_pages, new_mt);
+
+       if (old_isolated != new_isolated) {
+               nr_pages = old_isolated ? nr_pages : -nr_pages;
+               __mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
+               if (order >= pageblock_order)
+                       __mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS,
+                                             nr_pages);
        }
 }
 
I don't think it matters that we reorder the __mod_zone_page_state to be
after the account_specific_freepages here, so hopefully it is OK here.

So we can achieve the best of both worlds by preventing the duplicate adjustment
and also keep the control flow simple! (We can also just include that
additional check inside your account_freepages_both as well).

This is just my small idea : -) Of course, please feel free to ignore it if
you feel that it makes the code more confusing. I think that what is "simple"
is mostly subjective, so this was just my thought.

Thank you for your thoughts, I hope you have a great day!
Joshua

Re: [PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[Yajun Deng]

> 2026年1月11日 22:24，Joshua Hahn <joshua.hahnjy@gmail.com> 写道：
> 
> On Sun, 11 Jan 2026 21:47:42 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:
> 
>> 
>> 
>>> 2026年1月10日 00:31，Joshua Hahn <joshua.hahnjy@gmail.com> 写道：
>>> 
>>> On Fri,  9 Jan 2026 18:51:21 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:
>>> 
>>>> In move_to_free_list(), when a page block changes its migration type,
>>>> we need to update free page counts for both the old and new types.
>>>> Originally, this was done by two calls to account_freepages(), which
>>>> updates NR_FREE_PAGES and also type-specific counters. However, this
>>>> causes NR_FREE_PAGES to be updated twice, while the net change is zero
>>>> in most cases.
>>>> 
>>>> This patch introduces a new function account_freepages_both() that
>>>> updates the statistics for both old and new migration types in one go.
>>>> It avoids the double update of NR_FREE_PAGES by computing the net change
>>>> only when the isolation status changes.
>>>> 
>>>> The optimization avoid duplicate NR_FREE_PAGES updates in
>>>> move_to_free_list().
>>> 
>>> Hi Yajun,
>>> 
>>> I hope you are doing well, thank you for the patch! I was hoping to better
>>> understand the motivation behind this patch.
>>> 
>>> From my perspective, I believe that the current state of the code is
>>> not optimal, but it is also not problematic. account_freepages seems like
>>> a relatively cheap function (at the core, it's just some atomic operations).
>>> Personally I also think that semantically, the code currently makes sense;
>>> we are doing the accounting for the old mounttype, then for the new mounttype,
>>> in a way that cancels out. And given that there is still some cases where
>>> the work doesn't end up canceling out due to one of the mounttypes being
>>> MIGRATE_ISOLATE, I think that there is enough purpose in making the two
>>> calls to do the accounting twice.
>>> 
>>> On the other hand I think there is only one place in the codebase that
>>> will use account_freepages_both, so it might make the burden to understand
>>> the code a bit higher.
>>> 
>>> What do you think? I don't have a strong stance on whether the performance
>>> effects are big here (if this change indeed has a big performance implication,
>>> then we should definitely go forth with this!) but I do believe the current
>>> code is quite semantically sound and more readable. 
>>> 
>> Hey Joshua,
>> 
>> Thank you for sharing your thoughts. 
>> 
>> I currently don’t have any performance data, I just noticed from looking at the code
>> that there may be room for optimization.
>> You’re right. The original code is indeed more straightforward. I think we can add some
>> comments in the  account_freepages_both to make it easier to understand.
> 
> Hi Yajun, I hope you are doing well!
> 
> On second thought, I did notice that at the end of move_to_free_list, we have
> some additional conditionals that depend on the migratetype of the mounttypes.
> 
> What if we open-code the account_freepages_both, and skip doing the
> isolation checks twice? Your idea to use the ternary operator gave me this idea!
> 
> @@ -869,14 +877,17 @@ static inline void move_to_free_list(struct page *page, struct zone *zone,
> 
>        list_move_tail(&page->buddy_list, &area->free_list[new_mt]);
> 
> -       account_freepages(zone, -nr_pages, old_mt);
> -       account_freepages(zone, nr_pages, new_mt);
> -
> -       if (order >= pageblock_order &&
> -           is_migrate_isolate(old_mt) != is_migrate_isolate(new_mt)) {
> -               if (!is_migrate_isolate(old_mt))
> -                       nr_pages = -nr_pages;
> -               __mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS, nr_pages);
> +       if (!old_isolated)
> +               account_specific_freepages(zone, -nr_pages, old_mt);
> +       if !(new_isolated)
> +               account_specific_freepages(zone, nr_pages, new_mt);
> +
> +       if (old_isolated != new_isolated) {
> +               nr_pages = old_isolated ? nr_pages : -nr_pages;
> +               __mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
> +               if (order >= pageblock_order)
> +                       __mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS,
> +                                             nr_pages);
>        }
> }
> 
> I don't think it matters that we reorder the __mod_zone_page_state to be
> after the account_specific_freepages here, so hopefully it is OK here.
> 
> So we can achieve the best of both worlds by preventing the duplicate adjustment
> and also keep the control flow simple! (We can also just include that
> additional check inside your account_freepages_both as well).
> 
> This is just my small idea : -) Of course, please feel free to ignore it if
> you feel that it makes the code more confusing. I think that what is "simple"
> is mostly subjective, so this was just my thought.
> 

I think this is a good idea and I will adopt it.
Thank you

> Thank you for your thoughts, I hope you have a great day!
> Joshua

Re: [PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[Andrew Morton]

On Fri,  9 Jan 2026 18:51:21 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:

> In move_to_free_list(), when a page block changes its migration type,
> we need to update free page counts for both the old and new types.
> Originally, this was done by two calls to account_freepages(), which
> updates NR_FREE_PAGES and also type-specific counters. However, this
> causes NR_FREE_PAGES to be updated twice, while the net change is zero
> in most cases.
> 
> This patch introduces a new function account_freepages_both() that
> updates the statistics for both old and new migration types in one go.
> It avoids the double update of NR_FREE_PAGES by computing the net change
> only when the isolation status changes.
> 
> The optimization avoid duplicate NR_FREE_PAGES updates in
> move_to_free_list().

Seems nice and LGTM.

> +static inline void account_freepages_both(struct zone *zone, int nr_pages,
> +					  int old_mt, int new_mt)
> +{
> +	lockdep_assert_held(&zone->lock);
> +
> +	bool old_isolated = is_migrate_isolate(old_mt);
> +	bool new_isolated = is_migrate_isolate(new_mt);

We do permit C99 definition ordering nowadays, but I do think our eyes
and brains prefer the old-school style.

So here I'd personally prefer

	bool old_isolated = is_migrate_isolate(old_mt);
	bool new_isolated = is_migrate_isolate(new_mt);

	lockdep_assert_held(&zone->lock);


Or simply remove the assertion - it doesn't look useful to me.  If we
aren't holding zone->lock here then the kernel is so screwed up we
should all just go home.

Re: [PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list()

[Yajun Deng]

> 2026年1月11日 08:10，Andrew Morton <akpm@linux-foundation.org> 写道：
> 
> On Fri,  9 Jan 2026 18:51:21 +0800 Yajun Deng <yajun.deng@linux.dev> wrote:
> 
>> In move_to_free_list(), when a page block changes its migration type,
>> we need to update free page counts for both the old and new types.
>> Originally, this was done by two calls to account_freepages(), which
>> updates NR_FREE_PAGES and also type-specific counters. However, this
>> causes NR_FREE_PAGES to be updated twice, while the net change is zero
>> in most cases.
>> 
>> This patch introduces a new function account_freepages_both() that
>> updates the statistics for both old and new migration types in one go.
>> It avoids the double update of NR_FREE_PAGES by computing the net change
>> only when the isolation status changes.
>> 
>> The optimization avoid duplicate NR_FREE_PAGES updates in
>> move_to_free_list().
> 
> Seems nice and LGTM.
> 
>> +static inline void account_freepages_both(struct zone *zone, int nr_pages,
>> +  int old_mt, int new_mt)
>> +{
>> + lockdep_assert_held(&zone->lock);
>> +
>> + bool old_isolated = is_migrate_isolate(old_mt);
>> + bool new_isolated = is_migrate_isolate(new_mt);
> 
> We do permit C99 definition ordering nowadays, but I do think our eyes
> and brains prefer the old-school style.
> 
> So here I'd personally prefer
> 
> bool old_isolated = is_migrate_isolate(old_mt);
> bool new_isolated = is_migrate_isolate(new_mt);
> 
> lockdep_assert_held(&zone->lock);
> 
> 
> Or simply remove the assertion - it doesn't look useful to me.  If we
> aren't holding zone->lock here then the kernel is so screwed up we
> should all just go home.
> 
> 
Okay, I’ll remove the assertion.

Thanks.
>