Re: [PATCH] mm/oom: added option 'oom_dump_task_cmdline'

[Michal Hocko <mhocko@kernel.org>]

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.
 
> Signed-off-by: Stepan Bujnak <stepan@pex.com>
> ---
>  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.

-- 
Michal Hocko
SUSE Labs