Re: [PATCH] mm: memcg: Fix memcg reclaim soft lockup
From: xunlei <hidden>
Date: 2020-08-26 10:41:31
Also in:
lkml
On 2020/8/26 下午4:11, Michal Hocko wrote:
On Wed 26-08-20 15:27:02, Xunlei Pang wrote:quoted
We've met softlockup with "CONFIG_PREEMPT_NONE=y", when the target memcg doesn't have any reclaimable memory.Do you have any scenario when this happens or is this some sort of a test case?
It can happen on tiny guest scenarios.
quoted hunk ↗ jump to hunk
quoted
It can be easily reproduced as below: watchdog: BUG: soft lockup - CPU#0 stuck for 111s![memcg_test:2204] CPU: 0 PID: 2204 Comm: memcg_test Not tainted 5.9.0-rc2+ #12 Call Trace: shrink_lruvec+0x49f/0x640 shrink_node+0x2a6/0x6f0 do_try_to_free_pages+0xe9/0x3e0 try_to_free_mem_cgroup_pages+0xef/0x1f0 try_charge+0x2c1/0x750 mem_cgroup_charge+0xd7/0x240 __add_to_page_cache_locked+0x2fd/0x370 add_to_page_cache_lru+0x4a/0xc0 pagecache_get_page+0x10b/0x2f0 filemap_fault+0x661/0xad0 ext4_filemap_fault+0x2c/0x40 __do_fault+0x4d/0xf9 handle_mm_fault+0x1080/0x1790 It only happens on our 1-vcpu instances, because there's no chance for oom reaper to run to reclaim the to-be-killed process. Add cond_resched() in such cases at the beginning of shrink_lruvec() to give up the cpu to others.I do agree that we need a cond_resched but I cannot say I would like this patch. The primary reason is that it doesn't catch all cases when the memcg is not reclaimable. For example it wouldn't reschedule if the memcg is protected by low/min. What do you think about this instead?diff --git a/mm/vmscan.c b/mm/vmscan.c index 99e1796eb833..bbdc38b58cc5 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c@@ -2617,6 +2617,8 @@ static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc) mem_cgroup_calculate_protection(target_memcg, memcg); + cond_resched(); + if (mem_cgroup_below_min(memcg)) { /* * Hard protection.This should catch both cases. I even have a vague recollection that somebody has proposed something in that direction but I cannot remember what has happened with that patch.
It's the endless "retry" in try_charge() that caused the softlockup, and I think mem_cgroup_protected() will eventually return MEMCG_PROT_NONE, and shrink_node_memcgs() will call shrink_lruvec() for memcg self-reclaim cases, so it's not a problem here. But adding cond_resched() at upper shrink_node_memcgs() may eliminate potential similar issues, I have no objection with this approach. I tested it and works well, will send v2 later.