Re: [PATCH] mm: memcg: Fix memcg reclaim soft lockup

[Michal Hocko <mhocko@suse.com>]

On Wed 26-08-20 15:27:02, Xunlei Pang wrote:
> 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 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.
-- 
Michal Hocko
SUSE Labs