Re: [RFC 0/4] mm: zswap: add support for zswapin of large folios

Yosry Ahmed
2024-10-23T18:03:28Z

[..]
> >> I suspect the regression occurs because you're running an edge case
> >> where the memory cgroup stays nearly full most of the time (this isn't
> >> an inherent issue with large folio swap-in). As a result, swapping in
> >> mTHP quickly triggers a memcg overflow, causing a swap-out. The
> >> next swap-in then recreates the overflow, leading to a repeating
> >> cycle.
> >>
> >
> > Yes, agreed! Looking at the swap counters, I think this is what is going
> > on as well.
> >
> >> We need a way to stop the cup from repeatedly filling to the brim and
> >> overflowing. While not a definitive fix, the following change might help
> >> improve the situation:
> >>
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >>
> >> index 17af08367c68..f2fa0eeb2d9a 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >>
> >> @@ -4559,7 +4559,10 @@ int mem_cgroup_swapin_charge_folio(struct folio
> >> *folio, struct mm_struct *mm,
> >> memcg = get_mem_cgroup_from_mm(mm);
> >> rcu_read_unlock();
> >>
> >> - ret = charge_memcg(folio, memcg, gfp);
> >> + if (folio_test_large(folio) && mem_cgroup_margin(memcg) <
> >> MEMCG_CHARGE_BATCH)
> >> + ret = -ENOMEM;
> >> + else
> >> + ret = charge_memcg(folio, memcg, gfp);
> >>
> >> css_put(&memcg->css);
> >> return ret;
> >> }
> >>
> >
> > The diff makes sense to me. Let me test later today and get back to you.
> >
> > Thanks!
> >
> >> Please confirm if it makes the kernel build with memcg limitation
> >> faster. If so, let's
> >> work together to figure out an official patch :-) The above code hasn't consider
> >> the parent memcg's overflow, so not an ideal fix.
> >>
>
> Thanks Barry, I think this fixes the regression, and even gives an improvement!
> I think the below might be better to do:
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c098fd7f5c5e..0a1ec55cc079 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4550,7 +4550,11 @@ int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_struct *mm,
> memcg = get_mem_cgroup_from_mm(mm);
> rcu_read_unlock();
>
> - ret = charge_memcg(folio, memcg, gfp);
> + if (folio_test_large(folio) &&
> + mem_cgroup_margin(memcg) < max(MEMCG_CHARGE_BATCH, folio_nr_pages(folio)))
> + ret = -ENOMEM;
> + else
> + ret = charge_memcg(folio, memcg, gfp);
>
> css_put(&memcg->css);
> return ret;
>
>
> AMD 16K+32K THP=always
> metric mm-unstable mm-unstable + large folio zswapin series mm-unstable + large folio zswapin + no swap thrashing fix
> real 1m23.038s 1m23.050s 1m22.704s
> user 53m57.210s 53m53.437s 53m52.577s
> sys 7m24.592s 7m48.843s 7m22.519s
> zswpin 612070 999244 815934
> zswpout 2226403 2347979 2054980
> pgfault 20667366 20481728 20478690
> pgmajfault 385887 269117 309702
>
> AMD 16K+32K+64K THP=always
> metric mm-unstable mm-unstable + large folio zswapin series mm-unstable + large folio zswapin + no swap thrashing fix
> real 1m22.975s 1m23.266s 1m22.549s
> user 53m51.302s 53m51.069s 53m46.471s
> sys 7m40.168s 7m57.104s 7m25.012s
> zswpin 676492 1258573 1225703
> zswpout 2449839 2714767 2899178
> pgfault 17540746 17296555 17234663
> pgmajfault 429629 307495 287859
>

Thanks Usama and Barry for looking into this. It seems like this would
fix a regression with large folio swapin regardless of zswap. Can the
same result be reproduced on zram without this series?