[PATCH] mm/migrate: don't move memcg NR_FILE_PAGES for swapcache

[PATCH] mm/migrate: don't move memcg NR_FILE_PAGES for swapcache

[Song Hu]

NR_FILE_PAGES lives on both the node and the memcg plane for page
cache folios: filemap_add_folio() and filemap_unaccount_folio() charge
it to the folio's node and memcg, while __swap_cache_add_folio() and
__swap_cache_del_folio() maintain the node plane of NR_FILE_PAGES and
the memcg plane of NR_SWAPCACHE.  A folio in the swap cache therefore
carries no per-memcg NR_FILE_PAGES count: shmem folios drop their
memcg file accounting when swapped out, and anon folios never had any.

The cross-zone arm of __folio_migrate_mapping() moves NR_FILE_PAGES on
both planes unconditionally.  For swapcache folios the memcg plane is
empty, so migrating one misaccounts it: the destination accrues
nr_pages that were never accounted there while the source counter
underflows.  The wrong counts persist until the memcg is destroyed,
but the aggregate in memory.stat stays correct, so the drift is easy
to miss.  NUMA balancing, migratepages, compaction and memory hotplug
can all migrate such folios.

Reproducer, on a 2-node QEMU guest with cgroup v2:

  1. create a cgroup with memory.max = 50M and swap enabled;
  2. memset 128M of anonymous memory in it, so reclaim swaps most of
     it out;
  3. fork to raise the swap entry refcount, then fault the memory
     back in, so the resident folios are swapcache ones;
  4. migrate_pages(pid, node0 -> node1).

"file" row of memory.numa_stat (bytes) after step 4:

                    without this patch      with this patch
                      N0          N1          N0          N1
                     0 (*)  51691520         0           0

  (*) the source counter has gone negative; negative values read as 0.

Keep moving the node plane via mod_node_page_state() and skip only
the empty memcg plane, mirroring the swapcache exclusion the NR_SHMEM
handling in the same block already applies.  It accompanies
commit 4eef27af2932 ("mm/migrate: move NR_FILE_THPS on cross-node migration")
and commit c5eef2e03618 ("mm/migrate: move NR_KERNEL_FILE_PAGES on cross-node migration").

Fixes: 0d1c20722ab3 ("mm: memcontrol: switch to native NR_FILE_PAGES and NR_SHMEM counters")
Signed-off-by: Song Hu <husong@kylinos.cn>
---
 mm/migrate.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 28a8d7734735..bc60f6587d1b 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -690,8 +690,20 @@ static int __folio_migrate_mapping(struct address_space *mapping,
 		old_lruvec = mem_cgroup_lruvec(memcg, oldzone->zone_pgdat);
 		new_lruvec = mem_cgroup_lruvec(memcg, newzone->zone_pgdat);
 
-		mod_lruvec_state(old_lruvec, NR_FILE_PAGES, -nr);
-		mod_lruvec_state(new_lruvec, NR_FILE_PAGES, nr);
+		/*
+		 * Swapcache folios carry NR_FILE_PAGES on the node plane
+		 * only (their memcg footprint is NR_SWAPCACHE, see
+		 * swap_state.c), so move only the node plane for them.
+		 */
+		if (folio_test_swapcache(folio)) {
+			mod_node_page_state(oldzone->zone_pgdat, NR_FILE_PAGES,
+					    -nr);
+			mod_node_page_state(newzone->zone_pgdat, NR_FILE_PAGES,
+					    nr);
+		} else {
+			mod_lruvec_state(old_lruvec, NR_FILE_PAGES, -nr);
+			mod_lruvec_state(new_lruvec, NR_FILE_PAGES, nr);
+		}
 		if (folio_test_swapbacked(folio) && !folio_test_swapcache(folio)) {
 			mod_lruvec_state(old_lruvec, NR_SHMEM, -nr);
 			mod_lruvec_state(new_lruvec, NR_SHMEM, nr);
-- 
2.53.0

Re: [PATCH] mm/migrate: don't move memcg NR_FILE_PAGES for swapcache

[Joshua Hahn]

On Tue, Nov 3, 2026 at 11:43 PM Song Hu <husong@kylinos.cn> wrote:
>
> NR_FILE_PAGES lives on both the node and the memcg plane for page
> cache folios: filemap_add_folio() and filemap_unaccount_folio() charge
> it to the folio's node and memcg, while __swap_cache_add_folio() and
> __swap_cache_del_folio() maintain the node plane of NR_FILE_PAGES and
> the memcg plane of NR_SWAPCACHE.  A folio in the swap cache therefore
> carries no per-memcg NR_FILE_PAGES count: shmem folios drop their
> memcg file accounting when swapped out, and anon folios never had any.
>
> The cross-zone arm of __folio_migrate_mapping() moves NR_FILE_PAGES on
> both planes unconditionally.  For swapcache folios the memcg plane is
> empty, so migrating one misaccounts it: the destination accrues
> nr_pages that were never accounted there while the source counter
> underflows.  The wrong counts persist until the memcg is destroyed,
> but the aggregate in memory.stat stays correct, so the drift is easy
> to miss.  NUMA balancing, migratepages, compaction and memory hotplug
> can all migrate such folios.

Hi Song, thanks for the patch!

I think this description was a little bit hard to read @_@ so the core
problem is that swapcache folios are accounted differently in per-node
and per-memcg, right? And that swapcache is not accounted towards
memcg's NR_FILE_PAGES, so we need to split up the mod_lruvec_state
into the node and memcg accounting separately. This makes sense to me.

> Reproducer, on a 2-node QEMU guest with cgroup v2:
>
>   1. create a cgroup with memory.max = 50M and swap enabled;
>   2. memset 128M of anonymous memory in it, so reclaim swaps most of
>      it out;
>   3. fork to raise the swap entry refcount, then fault the memory
>      back in, so the resident folios are swapcache ones;
>   4. migrate_pages(pid, node0 -> node1).
>
> "file" row of memory.numa_stat (bytes) after step 4:
>
>                     without this patch      with this patch
>                       N0          N1          N0          N1
>                      0 (*)  51691520         0           0
>
>   (*) the source counter has gone negative; negative values read as 0.

Thanks for adding the reproducer. This helps a lot for reviewing!

> Keep moving the node plane via mod_node_page_state() and skip only
> the empty memcg plane, mirroring the swapcache exclusion the NR_SHMEM
> handling in the same block already applies.  It accompanies

So you note here that the block already has a kind of swapcache
exclusion. To me I think the code block we are working with here is a
little bit complicated with all the cases we have to look into (and
three separate folio_test_swapcache() calls...). At a future time I
think we should explicitly lay out all the cases (swapcache /
non-swapcache shmem / normal file) to make it a bit easier to follow
where everything gets accounted for.

I say this because it took me a second to realize that the #ifdef
CONFIG_SWAP should only apply to the NR_SWAPCACHE modification and not
other swapcache conditionals since it protects the enum and nothing
else.

Anyways, none of that should block this clearly user-visible bug.
Thanks! I hope you have a great day,
Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>

> commit 4eef27af2932 ("mm/migrate: move NR_FILE_THPS on cross-node migration")
> and commit c5eef2e03618 ("mm/migrate: move NR_KERNEL_FILE_PAGES on cross-node migration").
>
> Fixes: 0d1c20722ab3 ("mm: memcontrol: switch to native NR_FILE_PAGES and NR_SHMEM counters")
> Signed-off-by: Song Hu <husong@kylinos.cn>
> ---
>  mm/migrate.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 28a8d7734735..bc60f6587d1b 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -690,8 +690,20 @@ static int __folio_migrate_mapping(struct address_space *mapping,
>                 old_lruvec = mem_cgroup_lruvec(memcg, oldzone->zone_pgdat);
>                 new_lruvec = mem_cgroup_lruvec(memcg, newzone->zone_pgdat);
>
> -               mod_lruvec_state(old_lruvec, NR_FILE_PAGES, -nr);
> -               mod_lruvec_state(new_lruvec, NR_FILE_PAGES, nr);
> +               /*
> +                * Swapcache folios carry NR_FILE_PAGES on the node plane
> +                * only (their memcg footprint is NR_SWAPCACHE, see
> +                * swap_state.c), so move only the node plane for them.
> +                */
> +               if (folio_test_swapcache(folio)) {
> +                       mod_node_page_state(oldzone->zone_pgdat, NR_FILE_PAGES,
> +                                           -nr);
> +                       mod_node_page_state(newzone->zone_pgdat, NR_FILE_PAGES,
> +                                           nr);
> +               } else {
> +                       mod_lruvec_state(old_lruvec, NR_FILE_PAGES, -nr);
> +                       mod_lruvec_state(new_lruvec, NR_FILE_PAGES, nr);
> +               }
>                 if (folio_test_swapbacked(folio) && !folio_test_swapcache(folio)) {
>                         mod_lruvec_state(old_lruvec, NR_SHMEM, -nr);
>                         mod_lruvec_state(new_lruvec, NR_SHMEM, nr);
> --
> 2.53.0
>

Re: [PATCH] mm/migrate: don't move memcg NR_FILE_PAGES for swapcache

[David Hildenbrand (Arm)]

On 11/4/26 16:54, Joshua Hahn wrote:
> On Tue, Nov 3, 2026 at 11:43 PM Song Hu <husong@kylinos.cn> wrote:
>>
>> NR_FILE_PAGES lives on both the node and the memcg plane for page
>> cache folios: filemap_add_folio() and filemap_unaccount_folio() charge
>> it to the folio's node and memcg, while __swap_cache_add_folio() and
>> __swap_cache_del_folio() maintain the node plane of NR_FILE_PAGES and
>> the memcg plane of NR_SWAPCACHE.  A folio in the swap cache therefore
>> carries no per-memcg NR_FILE_PAGES count: shmem folios drop their
>> memcg file accounting when swapped out, and anon folios never had any.
>>
>> The cross-zone arm of __folio_migrate_mapping() moves NR_FILE_PAGES on
>> both planes unconditionally.  For swapcache folios the memcg plane is
>> empty, so migrating one misaccounts it: the destination accrues
>> nr_pages that were never accounted there while the source counter
>> underflows.  The wrong counts persist until the memcg is destroyed,
>> but the aggregate in memory.stat stays correct, so the drift is easy
>> to miss.  NUMA balancing, migratepages, compaction and memory hotplug
>> can all migrate such folios.
> 
> Hi Song, thanks for the patch!
> 
> I think this description was a little bit hard to read @_@ so the core
> problem is that swapcache folios are accounted differently in per-node
> and per-memcg, right? And that swapcache is not accounted towards
> memcg's NR_FILE_PAGES, so we need to split up the mod_lruvec_state
> into the node and memcg accounting separately. This makes sense to me.
> 
>> Reproducer, on a 2-node QEMU guest with cgroup v2:
>>
>>   1. create a cgroup with memory.max = 50M and swap enabled;
>>   2. memset 128M of anonymous memory in it, so reclaim swaps most of
>>      it out;
>>   3. fork to raise the swap entry refcount, then fault the memory
>>      back in, so the resident folios are swapcache ones;
>>   4. migrate_pages(pid, node0 -> node1).
>>
>> "file" row of memory.numa_stat (bytes) after step 4:
>>
>>                     without this patch      with this patch
>>                       N0          N1          N0          N1
>>                      0 (*)  51691520         0           0
>>
>>   (*) the source counter has gone negative; negative values read as 0.
> 
> Thanks for adding the reproducer. This helps a lot for reviewing!
> 
>> Keep moving the node plane via mod_node_page_state() and skip only
>> the empty memcg plane, mirroring the swapcache exclusion the NR_SHMEM
>> handling in the same block already applies.  It accompanies
> 
> So you note here that the block already has a kind of swapcache
> exclusion. To me I think the code block we are working with here is a
> little bit complicated with all the cases we have to look into (and
> three separate folio_test_swapcache() calls...). At a future time I
> think we should explicitly lay out all the cases (swapcache /
> non-swapcache shmem / normal file) to make it a bit easier to follow
> where everything gets accounted for.
> 
> I say this because it took me a second to realize that the #ifdef
> CONFIG_SWAP should only apply to the NR_SWAPCACHE modification and not
> other swapcache conditionals since it protects the enum and nothing
> else.
> 
> Anyways, none of that should block this clearly user-visible bug.
> Thanks! I hope you have a great day,
> Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>

This sounds like something we want to fix in stable?

-- 
Cheers,

David