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

[Michal Hocko <mhocko@kernel.org>]

On Thu 16-07-20 19:53:12, Yafang Shao wrote:
[...]
> I'd prefer to put dump_header() behind do_send_sig_info(), for example,
> 
> __oom_kill_process()
>     do_send_sig_info()
>     dump_header() <<<< may better put it behind wake_oom_reaper(), but
> it may loses some information to dump...
>     pr_err("%s: Killed process %d (%s)....")
> 
> Because the main goal of OOM is to kill a process to free pages ASAP
> to avoid system stall or memcg stall.
> We all find that  dump_header() may take a long time to finish
> especially if there is a slow console, and this long time may cause a
> great system stall, so we'd better defer the dump of it.

I would be worried that such a memory dump would be very likely not
reflecting the actual OOM situation and thus hard to use for analysis.
Why? Because the killed oom victim can _usually_ die or the oom reaper
can free up a lot of memory very quickly.

If we look at it the main part of the report which takes long to dump is
dump_tasks because this is potentially a lot of data. I haven't seen
show_mem or its memcg counterpart to take a long time. We've discussed
how much dump_tasks is really useful for production systems and the
general feedback was that it should be enabled by default, though.

If the oom report should be misleading then it would be better to not
bother at all IMHO. From my experience the memory state is really useful
when debugging ooms and having a reasonable snapshot is really
important. IIRC Tetsuo was suggesting collecting all the information at
the time of the oom and print that snapshot later on but the patch was
quite invasive and didn't handle multiple OOMs from different oom
domains - especially the dump_task part would need to link all the tasks
for different oom contexts.

Anyway, I think we are getting off topic here.
-- 
Michal Hocko
SUSE Labs