On Thu, Mar 6, 2025 at 10:32 AM Muchun Song songmuchun@bytedance.com wrote:
The commit 6769183166b3 has removed the parameter of id from swap_cgroup_record() and get the memcg id from mem_cgroup_id(folio_memcg(folio)). However, the caller of it may update a different memcg's counter instead of folio_memcg(folio). E.g. in the caller of mem_cgroup_swapout(), @swap_memcg could be different with @memcg and update the counter of @swap_memcg, but swap_cgroup_record() records the wrong memcg's ID. When it is uncharged from __mem_cgroup_uncharge_swap(), the swap counter will leak since the wrong recorded ID. Fix it by bring the parameter of id back.
Fixes: 6769183166b3 ("mm/swap_cgroup: decouple swap cgroup recording and clearing") Cc: stable@vger.kernel.org Signed-off-by: Muchun Song songmuchun@bytedance.com
include/linux/swap_cgroup.h | 4 ++-- mm/memcontrol.c | 4 ++-- mm/swap_cgroup.c | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/linux/swap_cgroup.h b/include/linux/swap_cgroup.h index b5ec038069dab..91cdf12190a03 100644 --- a/include/linux/swap_cgroup.h +++ b/include/linux/swap_cgroup.h @@ -6,7 +6,7 @@
#if defined(CONFIG_MEMCG) && defined(CONFIG_SWAP)
-extern void swap_cgroup_record(struct folio *folio, swp_entry_t ent); +extern void swap_cgroup_record(struct folio *folio, unsigned short id, swp_entry_t ent); extern unsigned short swap_cgroup_clear(swp_entry_t ent, unsigned int nr_ents); extern unsigned short lookup_swap_cgroup_id(swp_entry_t ent); extern int swap_cgroup_swapon(int type, unsigned long max_pages); @@ -15,7 +15,7 @@ extern void swap_cgroup_swapoff(int type); #else
static inline -void swap_cgroup_record(struct folio *folio, swp_entry_t ent) +void swap_cgroup_record(struct folio *folio, unsigned short id, swp_entry_t ent) { }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a5d870fbb4321..a5ab603806fbb 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4988,7 +4988,7 @@ void mem_cgroup_swapout(struct folio *folio, swp_entry_t entry) mem_cgroup_id_get_many(swap_memcg, nr_entries - 1); mod_memcg_state(swap_memcg, MEMCG_SWAP, nr_entries);
swap_cgroup_record(folio, entry);
swap_cgroup_record(folio, mem_cgroup_id(swap_memcg), entry); folio_unqueue_deferred_split(folio); folio->memcg_data = 0;@@ -5050,7 +5050,7 @@ int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry) mem_cgroup_id_get_many(memcg, nr_pages - 1); mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
swap_cgroup_record(folio, entry);
swap_cgroup_record(folio, mem_cgroup_id(memcg), entry); return 0;} diff --git a/mm/swap_cgroup.c b/mm/swap_cgroup.c index be39078f255be..1007c30f12e2c 100644 --- a/mm/swap_cgroup.c +++ b/mm/swap_cgroup.c @@ -58,9 +58,11 @@ static unsigned short __swap_cgroup_id_xchg(struct swap_cgroup *map,
- entries must not have been charged
- @folio: the folio that the swap entry belongs to
*/
- @id: mem_cgroup ID to be recorded
- @ent: the first swap entry to be recorded
-void swap_cgroup_record(struct folio *folio, swp_entry_t ent) +void swap_cgroup_record(struct folio *folio, unsigned short id,
swp_entry_t ent){ unsigned int nr_ents = folio_nr_pages(folio); struct swap_cgroup *map; @@ -72,8 +74,7 @@ void swap_cgroup_record(struct folio *folio, swp_entry_t ent) map = swap_cgroup_ctrl[swp_type(ent)].map;
do {
old = __swap_cgroup_id_xchg(map, offset,mem_cgroup_id(folio_memcg(folio)));
old = __swap_cgroup_id_xchg(map, offset, id); VM_BUG_ON(old); } while (++offset != end);}
2.20.1
Good catch, Thanks!
Reviewed-by: Kairui Song kasong@tencent.com