Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-Id: <202003130015.02D0F9uT079462@www262.sakura.ne.jp>
Subject: Re: [patch] mm, oom: prevent soft lockup on memcg oom for UP systems
From:   Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To:     David Rientjes <rientjes@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Michal Hocko <mhocko@kernel.org>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org
MIME-Version: 1.0
Date:   Fri, 13 Mar 2020 09:15:09 +0900
References: <202003120012.02C0CEUB043533@www262.sakura.ne.jp> <alpine.DEB.2.21.2003121101030.158939@chino.kir.corp.google.com>
In-Reply-To: <alpine.DEB.2.21.2003121101030.158939@chino.kir.corp.google.com>
Content-Type: text/plain; charset="ISO-2022-JP"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

David Rientjes wrote:
> > By the way, will you share the reproducer (and how to use the reproducer) ?
> > 
> 
> On an UP kernel with swap disabled, you limit a memcg to 100MB and start 
> three processes that each fault 40MB attached to it.  Same reproducer as 
> the "mm, oom: make a last minute check to prevent unnecessary memcg oom 
> kills" patch except in that case there are two cores.
> 

I'm not a heavy memcg user. Please provide steps for reproducing your problem
in a "copy and pastable" way (e.g. bash script, C program).

> > @@ -1576,6 +1576,7 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> >  	 */
> >  	ret = should_force_charge() || out_of_memory(&oc);
> >  	mutex_unlock(&oom_lock);
> > +	schedule_timeout_killable(1);
> >  	return ret;
> >  }
> >  
> 
> If current was process chosen for oom kill, this would actually induce the 
> problem, not fix it.
> 

Why? Memcg OOM path allows using forced charge path if should_force_charge() == true.

Since your lockup report

  Call Trace:
   shrink_node+0x40d/0x7d0
   do_try_to_free_pages+0x13f/0x470
   try_to_free_mem_cgroup_pages+0x16d/0x230
   try_charge+0x247/0xac0
   mem_cgroup_try_charge+0x10a/0x220
   mem_cgroup_try_charge_delay+0x1e/0x40
   handle_mm_fault+0xdf2/0x15f0
   do_user_addr_fault+0x21f/0x420
   page_fault+0x2f/0x40

says that allocating thread was calling try_to_free_mem_cgroup_pages() from try_charge(),
allocating thread must be able to reach mem_cgroup_out_of_memory() from mem_cgroup_oom()
 from try_charge(). And actually

  Memory cgroup out of memory: Killed process 808 (repro) total-vm:41944kB, anon-rss:35344kB, file-rss:504kB, shmem-rss:0kB, UID:0 pgtables:108kB oom_score_adj:0

says that allocating thread did reach mem_cgroup_out_of_memory(). Then, allocating thread
must be able to sleep at mem_cgroup_out_of_memory() if schedule_timeout_killable(1) is
mem_cgroup_out_of_memory().

Also, if current process was chosen for OOM-kill, current process will be able to leave
try_charge() due to should_force_charge() == true, won't it?

Thus, how can "this would actually induce the problem, not fix it." happen?
If your problem is that something keeps allocating threads away from reaching
should_force_charge() check, please explain the mechanism. If that is explained,
I would agree that schedule_timeout_killable(1) in mem_cgroup_out_of_memory()
won't help.