On Tue, Apr 14, 2020 at 12:52:30PM +0800, Alex Shi wrote: > 在 2020/4/14 上午2:07, Johannes Weiner 写道: > > Plus, the overhead of tracking is tiny - 512k per G of swap (0.04%). > > > > Maybe we should just delete MEMCG_SWAP and unconditionally track swap > > entry ownership when the memory controller is enabled. I don't see a > > good reason not to, and it would simplify the entire swapin path, the > > LRU locking, and the page->mem_cgroup stabilization rules. > > > > Sorry for not follow you up, did you mean just remove the MEMCG_SWAP configuration > and keep the feature in default memcg? Yes. > That does can remove lrucare, but PageLRU lock scheme still fails since > we can not isolate the page during commit_charge, is that right? No, without lrucare the scheme works. Charges usually do: page->mem_cgroup = new; SetPageLRU(page); And so if you can TestClearPageLRU(), page->mem_cgroup is stable. lrucare charging is the exception: it changes page->mem_cgroup AFTER PageLRU has already been set, and even when it CANNOT acquire the PageLRU lock itself. It violates the rules. If we make MEMCG_SWAP mandatory, we always have cgroup records for swapped out pages. That means we can charge all swapin pages (incl. readahead pages) directly in __read_swap_cache_async(), before setting PageLRU on the new pages. Then we can delete lrucare. And then TestClearPageLRU() guarantees page->mem_cgroup is stable.