Re: [PATCH v2] memcg, oom: check memcg margin for parallel oom

From: David Rientjes <rientjes@google.com>
To: Yafang Shao <laoar.shao@gmail.com>
Cc: Michal Hocko <mhocko@kernel.org>,
	 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	 Andrew Morton <akpm@linux-foundation.org>,
	 Johannes Weiner <hannes@cmpxchg.org>,
	Linux MM <linux-mm@kvack.org>
Subject: Re: [PATCH v2] memcg, oom: check memcg margin for parallel oom
Date: Tue, 14 Jul 2020 20:18:08 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.23.453.2007142016240.2667860@chino.kir.corp.google.com> (raw)
In-Reply-To: <CALOAHbB3wHgUPVJvg6trwWpNzeM=atgvoJ4wzih0g0DFdmStYw@mail.gmail.com>

On Wed, 15 Jul 2020, Yafang Shao wrote:

> > It has successfully resolved it for several years in our kernel, we tried
> > an approach similiar to yours but saw many instances where memcg oom kills
> > continued to proceed even though the memcg information dumped to the
> > kernel log showed memory available.
> >
> > If this was a page or two that became available due to memory freeing,
> > it's not a significant difference.  Instead, if this races with an oom
> > notification and a process exiting or being SIGKILL'd, it becomes much
> > harder to explain to a user why their process was oom killed when there
> > are tens of megabytes of memory available as shown by the kernel log (the
> > freeing/exiting happened during a particularly long iteration of processes
> > attached to the memcg, for example).
> >
> > That's what motivated a change to moving this to out_of_memory() directly,
> > we found that it prevented even more unnecessary oom kills, which is a
> > very good thing.  It may only be easily observable and make a significant
> > difference at very large scale, however.
> >
> 
> Thanks for the clarification.
> 
> If it is the race which causes this issue and we want to reduce the
> race window, I don't know whether it is proper to check the memcg
> margin in out_of_memory() or  do it before calling do_send_sig_info().
> Because per my understanding, dump_header() always takes much more
> time than select_bad_process() especially if there're slow consoles.
> So the race might easily happen when doing dump_header()  or dumping
> other information, but if we check the memcg margin after dumping this
> oom info, it would be strange to dump so much oom logs without killing
> a process.
> 

Absolutely correct :)  In my proposed patch, we declare dump_header() as 
the "point of no return" since we don't want to dump oom kill information 
to the kernel log when nothing is actually killed.  We could abort at the 
very last minute, as you mention, but I think that may have an adverse 
impact on anything that cares about that log message.