mm/oom: added option 'oom_dump_task_cmdline'
PATCH 1/1 mm/oom: added option 'oom_dump_task_cmdline'
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.
--- 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(-)
Documentation/sysctl/vm.txt +10 -0
@@ -50,6 +50,7 @@ Currently, these files are in /proc/sys/vm: - nr_trim_pages (only if CONFIG_MMU=n) - numa_zonelist_order - oom_dump_tasks +- oom_dump_task_cmdline - oom_kill_allocating_task - overcommit_kbytes - overcommit_memory
@@ -639,6 +640,15 @@ The default value is 1 (enabled). ============================================================== +oom_dump_task_cmdline + +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. + +The default value is 0 (disabled). + +============================================================== + oom_kill_allocating_task This enables or disables killing the OOM-triggering task in
include/linux/oom.h +1 -0
@@ -125,6 +125,7 @@ extern struct task_struct *find_lock_task_mm(struct task_struct *p);/* sysctls */ extern int sysctl_oom_dump_tasks; +extern int sysctl_oom_dump_task_cmdline;extern int sysctl_oom_kill_allocating_task;extern int sysctl_panic_on_oom;#endif /* _INCLUDE_LINUX_OOM_H */
kernel/sysctl.c +7 -0
@@ -1288,6 +1288,13 @@ static struct ctl_table vm_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, + { + .procname = "oom_dump_task_cmdline", + .data = &sysctl_oom_dump_task_cmdline, + .maxlen = sizeof(sysctl_oom_dump_task_cmdline), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "overcommit_ratio", .data = &sysctl_overcommit_ratio,
mm/oom_kill.c +18 -2
@@ -41,6 +41,7 @@#include <linux/kthread.h> #include <linux/init.h> #include <linux/mmu_notifier.h> +#include <linux/string_helpers.h> #include <asm/tlb.h> #include "internal.h"
@@ -52,6 +53,7 @@int sysctl_panic_on_oom;int sysctl_oom_kill_allocating_task;int sysctl_oom_dump_tasks =1 ; +int sysctl_oom_dump_task_cmdline;/* * Serializes oom killer invocations (out_of_memory()) from all contexts to
@@ -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) {/*
@@ -414,16 +425,21 @@ static void dump_tasks(struct mem_cgroup *memcg, const nodemask_t *nodemask)* detached their mm's. There's no need to report * them; they can't be oom killed anyway. */ -continue ; + goto done; } + name = cmd ? cmd : task->comm; +pr_info ("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n" , task->pid,from_kuid (&init_user_ns,task_uid (task)), task->tgid, task->mm->total_vm,get_mm_rss (task->mm),mm_pgtables_bytes (task->mm),get_mm_counter (task->mm,MM_SWAPENTS ), - task->signal->oom_score_adj, task->comm); + task->signal->oom_score_adj, name);task_unlock (task); + +done: +kfree (cmd); }rcu_read_unlock (); }
Signature · 1 line
2.17.1
Randy Dunlap
reviewerHi,
Spell it out correctly (2 places):
Quoted 1 line · Stepan Bujnak
When
oom_dump_tasksis enabled, this option will try to display task
When oom_dump_task_cmdline is enabled,
Quoted 31 lines
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.
Signed-off-by Stepan Bujnak <[email protected]>--- 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(-)Documentation/sysctl/vm.txt +1
-0@@ -50,6 +50,7 @@ Currently, these files are in /proc/sys/vm: - nr_trim_pages (only if CONFIG_MMU=n) - numa_zonelist_order - oom_dump_tasks +- oom_dump_task_cmdline - oom_kill_allocating_task - overcommit_kbytes - overcommit_memory
Documentation/sysctl/vm.txt The default value is 1 (enabled). · line 645
Quoted 6 lines
Documentation/sysctl/vm.txt +3
-0@@ -639,6 +640,15 @@ The default value is 1 (enabled). ============================================================== +oom_dump_task_cmdline + +When oom_dump_tasks is enabled, this option will try to display task cmdline
When oom_dump_task_cmdline is enabled,
Quoted 29 lines
Documentation/sysctl/vm.txt +6
-0+instead of the command name in the system-wide task dump. + +The default value is 0 (disabled). + +============================================================== + oom_kill_allocating_task This enables or disables killing the OOM-triggering task ininclude/linux/oom.h +1
-0@@ -125,6 +125,7 @@ extern struct task_struct *find_lock_task_mm(struct task_struct *p);/* sysctls */ extern int sysctl_oom_dump_tasks; +extern int sysctl_oom_dump_task_cmdline;extern int sysctl_oom_kill_allocating_task;extern int sysctl_panic_on_oom;#endif /* _INCLUDE_LINUX_OOM_H */ kernel/sysctl.c +0
-0@@ -1288,6 +1288,13 @@ static struct ctl_table vm_table[] = { .mode = 0644, .proc_handler = proc_dointvec, },
kernel/sysctl.c static struct ctl_table vm_table[] = { · line 1300
Quoted 10 lines
kernel/sysctl.c +7
-0+ { + .procname = "oom_dump_task_cmdline", + .data = &sysctl_oom_dump_task_cmdline, + .maxlen = sizeof(sysctl_oom_dump_task_cmdline), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "overcommit_ratio", .data = &sysctl_overcommit_ratio,
thanks.
Signature · 1 line
~Randy
Quoted 4 lines · Randy Dunlap
Hi,
Spell it out correctly (2 places):
This is not a typo. It actually refers to the oom_dump_tasks option, in a sense that when that option is enabled,
this option (oom_dump_task_cmdline) additionally displays task cmdline instead of task name.
Quoted 91 lines · Stepan Bujnak
Quoted 1 line
When
oom_dump_tasksis enabled, this option will try to display taskWhen
oom_dump_task_cmdlineis enabled,Quoted 31 lines
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.
Signed-off-by Stepan Bujnak <[email protected]>--- 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(-)Documentation/sysctl/vm.txt +1
-0@@ -50,6 +50,7 @@ Currently, these files are in /proc/sys/vm: - nr_trim_pages (only if CONFIG_MMU=n) - numa_zonelist_order - oom_dump_tasks +- oom_dump_task_cmdline - oom_kill_allocating_task - overcommit_kbytes - overcommit_memoryDocumentation/sysctl/vm.txt The default value is 1 (enabled). · line 645
Quoted 6 lines
Documentation/sysctl/vm.txt +3
-0@@ -639,6 +640,15 @@ The default value is 1 (enabled). ============================================================== +oom_dump_task_cmdline + +When oom_dump_tasks is enabled, this option will try to display task cmdlineWhen
oom_dump_task_cmdlineis enabled,Quoted 29 lines
Documentation/sysctl/vm.txt +6
-0+instead of the command name in the system-wide task dump. + +The default value is 0 (disabled). + +============================================================== + oom_kill_allocating_task This enables or disables killing the OOM-triggering task ininclude/linux/oom.h +1
-0@@ -125,6 +125,7 @@ extern struct task_struct *find_lock_task_mm(struct task_struct *p);/* sysctls */ extern int sysctl_oom_dump_tasks; +extern int sysctl_oom_dump_task_cmdline;extern int sysctl_oom_kill_allocating_task;extern int sysctl_panic_on_oom;#endif /* _INCLUDE_LINUX_OOM_H */ kernel/sysctl.c +0
-0@@ -1288,6 +1288,13 @@ static struct ctl_table vm_table[] = { .mode = 0644, .proc_handler = proc_dointvec, },kernel/sysctl.c static struct ctl_table vm_table[] = { · line 1300
Quoted 10 lines
kernel/sysctl.c +7
-0+ { + .procname = "oom_dump_task_cmdline", + .data = &sysctl_oom_dump_task_cmdline, + .maxlen = sizeof(sysctl_oom_dump_task_cmdline), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "overcommit_ratio", .data = &sysctl_overcommit_ratio,thanks.
--
~Randy
Randy Dunlap
reviewerQuoted 10 lines · Stepan
Quoted 4 lines · Randy Dunlap
Hi,
Spell it out correctly (2 places):
This is not a typo. It actually refers to the
oom_dump_tasksoption, in a sense that when that option is enabled,
this option (oom_dump_task_cmdline) additionally displays task cmdline instead of task name.Quoted 1 line
OK, thanks for clarifying.
Quoted 90 lines
Quoted 90 lines · Stepan Bujnak
Quoted 1 line
When
oom_dump_tasksis enabled, this option will try to display taskWhen
oom_dump_task_cmdlineis enabled,Quoted 31 lines
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.
Signed-off-by Stepan Bujnak <[email protected]>--- 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(-)Documentation/sysctl/vm.txt +1
-0@@ -50,6 +50,7 @@ Currently, these files are in /proc/sys/vm: - nr_trim_pages (only if CONFIG_MMU=n) - numa_zonelist_order - oom_dump_tasks +- oom_dump_task_cmdline - oom_kill_allocating_task - overcommit_kbytes - overcommit_memoryDocumentation/sysctl/vm.txt The default value is 1 (enabled). · line 645
Quoted 6 lines
Documentation/sysctl/vm.txt +3
-0@@ -639,6 +640,15 @@ The default value is 1 (enabled). ============================================================== +oom_dump_task_cmdline + +When oom_dump_tasks is enabled, this option will try to display task cmdlineWhen
oom_dump_task_cmdlineis enabled,Quoted 29 lines
Documentation/sysctl/vm.txt +6
-0+instead of the command name in the system-wide task dump. + +The default value is 0 (disabled). + +============================================================== + oom_kill_allocating_task This enables or disables killing the OOM-triggering task ininclude/linux/oom.h +1
-0@@ -125,6 +125,7 @@ extern struct task_struct *find_lock_task_mm(struct task_struct *p);/* sysctls */ extern int sysctl_oom_dump_tasks; +extern int sysctl_oom_dump_task_cmdline;extern int sysctl_oom_kill_allocating_task;extern int sysctl_panic_on_oom;#endif /* _INCLUDE_LINUX_OOM_H */ kernel/sysctl.c +0
-0@@ -1288,6 +1288,13 @@ static struct ctl_table vm_table[] = { .mode = 0644, .proc_handler = proc_dointvec, },kernel/sysctl.c static struct ctl_table vm_table[] = { · line 1300
Quoted 10 lines
kernel/sysctl.c +7
-0+ { + .procname = "oom_dump_task_cmdline", + .data = &sysctl_oom_dump_task_cmdline, + .maxlen = sizeof(sysctl_oom_dump_task_cmdline), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "overcommit_ratio", .data = &sysctl_overcommit_ratio,thanks.
--
~Randy
Signature · 1 line
~Randy
Quoted 10 lines · Stepan Bujnak
When
oom_dump_tasksis 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.
Quoted 8 lines
Signed-off-by Stepan Bujnak <[email protected]>--- 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(-)
[...]
Quoted 9 lines
mm/oom_kill.c +2
-0@@ -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 ;
mm/oom_kill.c static void dump_tasks(struct mem_cgroup *memcg, const nodemask_t *nodemask) · line 423
Quoted 10 lines
mm/oom_kill.c +7
-0+/* +* 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.
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 10 lines · Michal Hocko
Quoted 8 lines · Stepan Bujnak
When
oom_dump_tasksis 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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 18 lines
Quoted 8 lines
Signed-off-by Stepan Bujnak <[email protected]>--- 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(-)[...]
Quoted 9 lines
mm/oom_kill.c +2
-0@@ -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 ;
Quoted 15 lines
mm/oom_kill.c static void dump_tasks(struct mem_cgroup *memcg, const nodemask_t *nodemask) · line 423
Quoted 10 lines
mm/oom_kill.c +7
-0+/* +* 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_cmdlineneedsmmap_semand 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?
Quoted 3 lines
--
Michal Hocko
SUSE Labs
Quoted 25 lines · Stepan
Quoted 9 lines
Quoted 9 lines
mm/oom_kill.c +2
-0@@ -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 ;Quoted 15 lines
mm/oom_kill.c static void dump_tasks(struct mem_cgroup *memcg, const nodemask_t *nodemask) · line 423
Quoted 10 lines
mm/oom_kill.c +7
-0+/* +* 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_cmdlineneedsmmap_semand the allocating context migh hold the lock already. So the patch is simply wrong.
Quoted 6 lines
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->commis often not descriptive enough. Would it help if instead of callingkstrdup_quotable_cmdline()which allocates the buffer on heap I calledget_cmdline()directly passing it stack-allocated buffer of certain size e.g. 256?
You made triple errors. First is that doing GFP_KERNEL allocation inside rcu_read_lock()/rcu_read_unlock() is not permitted. Second is that doing GFP_KERNEL allocation with oom_lock held is not permitted. Third is that somebody might be already holding p->mm->mmap_sem for write when get_cmdline() tries to hold it for read. That is, your patch can't work (even if you update your patch to use static buffer).
Quoted 1 line · Stepan
On Wed, Feb 20, 2019 at 7:49 AM Michal Hocko <[email protected]> wrote:
[...]
Quoted 6 lines
Quoted 5 lines
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_cmdlineneedsmmap_semand the allocating context migh hold the lock already. So the patch is simply wrong.
Quoted 6 lines
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->commis often not descriptive enough. Would it help if instead of callingkstrdup_quotable_cmdline()which allocates the buffer on heap I calledget_cmdline()directly passing it stack-allocated buffer of certain size e.g. 256?
No it wouldn't because get_cmdline take mmap_sem lock as already pointed out.
Please also note that the cmd line might be considered security/privacy sensitive information and dumping it to the log sounds like a bad idea in general.
Signature · 2 lines
Michal Hocko SUSE Labs