Hi Kairui, On Wed, Nov 22, 2023 at 9:33 AM Kairui Song <ryncsn@xxxxxxxxx> wrote: > > There are two different charges. Memcg charging and memcg swapin charging. > > The folio_put will do the memcg discharge, the corresponding memcg > > charge is in follio allocation. > > Hi Chris, > > I didn't get your idea here... By "memcg swapin charge", do you mean > "memory.swap.*"? And "memcg charging" means "memory.*"?. There is no Sorry I should have used the function name then there is no ambiguity. "memcg swapin charge" I mean function mem_cgroup_swapin_charge_folio(). This function will look up the swap entry and find the memcg by swap entry then charge to that memcg. > memcg charge related code in folio allocation (alloc_pages_mpol), > actually the mem_cgroup_swapin_charge_folio here is doing memcg charge > not memcg swapin charge. Swapin path actually need to uncharge > "memory.swap" by mem_cgroup_swapin_uncharge_swap in later part of this > function. I still think you have a bug there. Take this make up example: Let say the for loop runs 3 times and the 3rd time breaks out the for loop. The original code will call: filemap_get_folio() 3 times folio_put() 2 times mem_cgroup_swapin_charge_folio() 1 time. With your patch, it will call: filemap_get_folio() 3 times folio_put() 2 times mem_cgroup_swapin_charge_folio() 3 times. Do you see the behavior difference there? Chris