2019-06-12 18:11:33

by Joel Savitz

[permalink] [raw]
Subject: [RESEND PATCH v2] mm/oom_killer: Add task UID to info message on an oom kill

In the event of an oom kill, useful information about the killed
process is printed to dmesg. Users, especially system administrators,
will find it useful to immediately see the UID of the process.

In the following example, abuse_the_ram is the name of a program
that attempts to iteratively allocate all available memory until it is
stopped by force.

Current message:

Out of memory: Killed process 35389 (abuse_the_ram)
total-vm:133718232kB, anon-rss:129624980kB, file-rss:0kB,
shmem-rss:0kB

Patched message:

Out of memory: Killed process 2739 (abuse_the_ram),
total-vm:133880028kB, anon-rss:129754836kB, file-rss:0kB,
shmem-rss:0kB, UID 0


Suggested-by: David Rientjes <[email protected]>
Signed-off-by: Joel Savitz <[email protected]>
---
mm/oom_kill.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 3a2484884cfd..af2e3faa72a0 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -874,12 +874,13 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
*/
do_send_sig_info(SIGKILL, SEND_SIG_PRIV, victim, PIDTYPE_TGID);
mark_oom_victim(victim);
- pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",
+ pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB, UID %d\n",
message, task_pid_nr(victim), victim->comm,
K(victim->mm->total_vm),
K(get_mm_counter(victim->mm, MM_ANONPAGES)),
K(get_mm_counter(victim->mm, MM_FILEPAGES)),
- K(get_mm_counter(victim->mm, MM_SHMEMPAGES)));
+ K(get_mm_counter(victim->mm, MM_SHMEMPAGES)),
+ from_kuid(&init_user_ns, task_uid(victim)));
task_unlock(victim);

/*
--
2.18.1


2019-06-12 18:41:39

by Rafael Aquini

[permalink] [raw]
Subject: Re: [RESEND PATCH v2] mm/oom_killer: Add task UID to info message on an oom kill

On Wed, Jun 12, 2019 at 01:57:53PM -0400, Joel Savitz wrote:
> In the event of an oom kill, useful information about the killed
> process is printed to dmesg. Users, especially system administrators,
> will find it useful to immediately see the UID of the process.
>
> In the following example, abuse_the_ram is the name of a program
> that attempts to iteratively allocate all available memory until it is
> stopped by force.
>
> Current message:
>
> Out of memory: Killed process 35389 (abuse_the_ram)
> total-vm:133718232kB, anon-rss:129624980kB, file-rss:0kB,
> shmem-rss:0kB
>
> Patched message:
>
> Out of memory: Killed process 2739 (abuse_the_ram),
> total-vm:133880028kB, anon-rss:129754836kB, file-rss:0kB,
> shmem-rss:0kB, UID 0
>
>
> Suggested-by: David Rientjes <[email protected]>
> Signed-off-by: Joel Savitz <[email protected]>
> ---
> mm/oom_kill.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 3a2484884cfd..af2e3faa72a0 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -874,12 +874,13 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
> */
> do_send_sig_info(SIGKILL, SEND_SIG_PRIV, victim, PIDTYPE_TGID);
> mark_oom_victim(victim);
> - pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",
> + pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB, UID %d\n",
> message, task_pid_nr(victim), victim->comm,
> K(victim->mm->total_vm),
> K(get_mm_counter(victim->mm, MM_ANONPAGES)),
> K(get_mm_counter(victim->mm, MM_FILEPAGES)),
> - K(get_mm_counter(victim->mm, MM_SHMEMPAGES)));
> + K(get_mm_counter(victim->mm, MM_SHMEMPAGES)),
> + from_kuid(&init_user_ns, task_uid(victim)));
> task_unlock(victim);
>
> /*
> --
> 2.18.1
>
Acked-by: Rafael Aquini <[email protected]>

2019-06-13 16:32:22

by Michal Hocko

[permalink] [raw]
Subject: Re: [RESEND PATCH v2] mm/oom_killer: Add task UID to info message on an oom kill

On Wed 12-06-19 13:57:53, Joel Savitz wrote:
> In the event of an oom kill, useful information about the killed
> process is printed to dmesg. Users, especially system administrators,
> will find it useful to immediately see the UID of the process.

Could you be more specific please? We already print uid when dumping
eligible tasks so it is not overly hard to find that information in the
oom report. Well, except when dumping of eligible tasks is disabled. Is
this what you are after?

Please always be specific about usecases in the changelog. A terse
statement that something is useful doesn't tell much very often.

Thanks!
--
Michal Hocko
SUSE Labs

2019-06-13 16:58:39

by Andrew Morton

[permalink] [raw]
Subject: Re: [RESEND PATCH v2] mm/oom_killer: Add task UID to info message on an oom kill

On Wed, 12 Jun 2019 13:57:53 -0400 Joel Savitz <[email protected]>
wrote:

> In the event of an oom kill, useful information about the killed
> process is printed to dmesg. Users, especially system administrators,
> will find it useful to immediately see the UID of the process.
>
> In the following example, abuse_the_ram is the name of a program
> that attempts to iteratively allocate all available memory until it is
> stopped by force.
>
> Current message:
>
> Out of memory: Killed process 35389 (abuse_the_ram)
> total-vm:133718232kB, anon-rss:129624980kB, file-rss:0kB,
> shmem-rss:0kB
>
> Patched message:
>
> Out of memory: Killed process 2739 (abuse_the_ram),
> total-vm:133880028kB, anon-rss:129754836kB, file-rss:0kB,
> shmem-rss:0kB, UID 0

The other fields are name:value so it seems better to make the UID
field conform.

Also, there's no typesafe way of printing a uid_t (using the printk %p trick)
so yes, we have to assume its type. But assuming unsigned int is
better than assuming int!

So...



s/UID %d/UID:%u/ in printk

--- a/mm/oom_kill.c~mm-oom_killer-add-task-uid-to-info-message-on-an-oom-kill-fix
+++ a/mm/oom_kill.c
@@ -876,7 +876,7 @@ static void __oom_kill_process(struct ta
*/
do_send_sig_info(SIGKILL, SEND_SIG_PRIV, victim, PIDTYPE_TGID);
mark_oom_victim(victim);
- pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB, UID %d\n",
+ pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB, UID:%u\n",
message, task_pid_nr(victim), victim->comm,
K(victim->mm->total_vm),
K(get_mm_counter(victim->mm, MM_ANONPAGES)),
_

2019-09-22 19:15:46

by Rafael Aquini

[permalink] [raw]
Subject: Re: [RESEND PATCH v2] mm/oom_killer: Add task UID to info message on an oom kill

On Fri, Sep 20, 2019 at 05:13:40PM -0700, Andrew Morton wrote:
> On Thu, 13 Jun 2019 10:23:18 +0200 Michal Hocko <[email protected]> wrote:
>
> > On Wed 12-06-19 13:57:53, Joel Savitz wrote:
> > > In the event of an oom kill, useful information about the killed
> > > process is printed to dmesg. Users, especially system administrators,
> > > will find it useful to immediately see the UID of the process.
> >
> > Could you be more specific please? We already print uid when dumping
> > eligible tasks so it is not overly hard to find that information in the
> > oom report. Well, except when dumping of eligible tasks is disabled. Is
> > this what you are after?
> >
> > Please always be specific about usecases in the changelog. A terse
> > statement that something is useful doesn't tell much very often.
> >
>
> <crickets?>
> I'll add this to the chagnelog:
>
> : We already print uid when dumping eligible tasks so it is not overly hard
> : to find that information in the oom report. However this information is
> : unavailable then dumping of eligible tasks is disabled.
^^^^

Thanks Andrew! just a minor nit there: 's/then/when/'


Acked-by: Rafael Aquini <[email protected]>
>

2019-09-23 13:54:16

by Andrew Morton

[permalink] [raw]
Subject: Re: [RESEND PATCH v2] mm/oom_killer: Add task UID to info message on an oom kill

On Thu, 13 Jun 2019 10:23:18 +0200 Michal Hocko <[email protected]> wrote:

> On Wed 12-06-19 13:57:53, Joel Savitz wrote:
> > In the event of an oom kill, useful information about the killed
> > process is printed to dmesg. Users, especially system administrators,
> > will find it useful to immediately see the UID of the process.
>
> Could you be more specific please? We already print uid when dumping
> eligible tasks so it is not overly hard to find that information in the
> oom report. Well, except when dumping of eligible tasks is disabled. Is
> this what you are after?
>
> Please always be specific about usecases in the changelog. A terse
> statement that something is useful doesn't tell much very often.
>

<crickets?>

I'll add this to the chagnelog:

: We already print uid when dumping eligible tasks so it is not overly hard
: to find that information in the oom report. However this information is
: unavailable then dumping of eligible tasks is disabled.