On Wed, Feb 20, 2019 at 7:49 AM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
>
> On Wed 20-02-19 04:22:45, Stepan Bujnak wrote:
> > When oom_dump_tasks is enabled, this option will try to display task
> > cmdline instead of the command name in the system-wide task dump.
> >
> > This is useful in some cases e.g. on postgres server. If OOM killer is
> > invoked it will show a bunch of tasks called 'postgres'. With this
> > option enabled it will show additional information like the database
> > user, database name and what it is currently doing.
> >
> > Other example is python. Instead of just 'python' it will also show the
> > script name currently being executed.
>
> The size of OOM report output is quite large already and this will just
> add much more for some workloads and printing from this context is quite
> a problem already.
>
The option defaults to false so most workloads wouldn't be affected.
As an alternative the cmdline line can only be printed for the
victim task in the OOM summary.
> > Signed-off-by: Stepan Bujnak <stepan@xxxxxxx>
> > ---
> > Documentation/sysctl/vm.txt | 10 ++++++++++
> > include/linux/oom.h | 1 +
> > kernel/sysctl.c | 7 +++++++
> > mm/oom_kill.c | 20 ++++++++++++++++++--
> > 4 files changed, 36 insertions(+), 2 deletions(-)
> >
> [...]
> > @@ -404,9 +406,18 @@ static void dump_tasks(struct mem_cgroup *memcg, const nodemask_t *nodemask)
> > pr_info("[ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj name\n");
> > rcu_read_lock();
> > for_each_process(p) {
> > + char *name, *cmd = NULL;
> > +
> > if (oom_unkillable_task(p, memcg, nodemask))
> > continue;
> >
> > + /*
> > + * This needs to be done before calling find_lock_task_mm()
> > + * since both grab a task lock which would result in deadlock.
> > + */
> > + if (sysctl_oom_dump_task_cmdline)
> > + cmd = kstrdup_quotable_cmdline(p, GFP_KERNEL);
> > +
> > task = find_lock_task_mm(p);
> > if (!task) {
> > /*
> You are trying to allocate from the OOM context. That is a big no no.
> Not to mention that this is deadlock prone because get_cmdline needs
> mmap_sem and the allocating context migh hold the lock already. So the
> patch is simply wrong.
>
Thanks for the notes. I understand how allocating from OOM context
is a problem. However I still believe that this would be helpful
for debugging OOM kills since task->comm is often not descriptive
enough. Would it help if instead of calling kstrdup_quotable_cmdline()
which allocates the buffer on heap I called get_cmdline() directly
passing it stack-allocated buffer of certain size e.g. 256?
> --
> Michal Hocko
> SUSE Labs
