On 23/10/2024 19:02, Yosry Ahmed wrote: > [..] >>>> 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? Yes, its a regression in large folio swapin support regardless of zswap/zram. Need to do 3 tests, one with probably the below diff to remove large folio support, one with current upstream and one with upstream + swap thrashing fix. We only use zswap and dont have a zram setup (and I am a bit lazy to create one :)). Any zram volunteers to try this? diff --git a/mm/memory.c b/mm/memory.c index fecdd044bc0b..62f6b087beb3 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4124,6 +4124,8 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf) gfp_t gfp; int order; + goto fallback; + /* * If uffd is active for the vma we need per-page fault fidelity to * maintain the uffd semantics.