| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 19 Jun 2015 09:09:43 +0200
From: Michal Hocko <mhocko@...e.cz>
To: David Rientjes <rientjes@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] oom: Do not panic when OOM killer is sysrq triggered
On Thu 18-06-15 12:21:22, David Rientjes wrote:
> On Thu, 18 Jun 2015, Michal Hocko wrote:
>
> > OOM killer might be triggered externally via sysrq+f. This is supposed
>
> I'm not sure what you mean by externally? Perhaps "explicitly"?
OK, explicitly is better.
[...]
> s/panicing/panicking/
Fixed
> > While we are there also add a comment explaining why
> > sysctl_oom_kill_allocating_task doesn't apply to sysrq triggered OOM
> > killer even though there is no explicit check and we subtly rely
> > on current->mm being NULL for the context from which it is triggered.
> >
> > Also be more explicit about sysrq+f behavior in the documentation.
> >
> > Requested-by: David Rientjes <rientjes@...gle.com>
> > Signed-off-by: Michal Hocko <mhocko@...e.cz>
> > ---
> > Documentation/sysrq.txt | 5 ++++-
> > mm/oom_kill.c | 21 +++++++++++++++++----
> > 2 files changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/sysrq.txt b/Documentation/sysrq.txt
> > index 0e307c94809a..a5dd88b0aede 100644
> > --- a/Documentation/sysrq.txt
> > +++ b/Documentation/sysrq.txt
> > @@ -75,7 +75,10 @@ On other - If you know of the key combos for other architectures, please
> >
> > 'e' - Send a SIGTERM to all processes, except for init.
> >
> > -'f' - Will call oom_kill to kill a memory hog process.
> > +'f' - Will call oom_kill to kill a memory hog process. Please note that
> > + an ongoing OOM killer is ignored and a task is killed even though
> > + there was an oom victim selected already. panic_on_oom is ignored
> > + and the system doesn't panic if there are no oom killable tasks.
>
> "an ongoing OOM killer" could probably be reworded to "parallel oom
> killings".
OK
> >
> > 'g' - Used by kgdb (kernel debugger)
> >
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index dff991e0681e..0c312eaac834 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -687,8 +687,14 @@ bool out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
> > constraint = constrained_alloc(zonelist, gfp_mask, nodemask,
> > &totalpages);
> > mpol_mask = (constraint == CONSTRAINT_MEMORY_POLICY) ? nodemask : NULL;
> > - check_panic_on_oom(constraint, gfp_mask, order, mpol_mask, NULL);
> > + /* Ignore panic_on_oom when the OOM killer is sysrq triggered */
> > + if (!force_kill)
> > + check_panic_on_oom(constraint, gfp_mask, order, mpol_mask, NULL);
>
> I don't think the comment is necessary, it should be clear from the code
> that this only executes when force_kill == true.
OK, I will remove the comment.
> You may want to reconsider my suggestion of renaming the formal as
> "sysrq".
I consider the naming clear enough
> >
> > + /*
> > + * not affecting force_kill because sysrq triggered OOM killer runs from
> > + * the workqueue context so current->mm will be NULL
> > + */
>
> Unnecessary comment, nobody is reading this code with the short circuit in
> mind.
I find this comment helpful because it is pointing a non trivial fact
that requires wandering the code otherwise. So I will keep it.
> > if (sysctl_oom_kill_allocating_task && current->mm &&
> > !oom_unkillable_task(current, NULL, nodemask) &&
> > current->signal->oom_score_adj != OOM_SCORE_ADJ_MIN) {
> > @@ -700,10 +706,17 @@ bool out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
> > }
> >
> > p = select_bad_process(&points, totalpages, mpol_mask, force_kill);
> > - /* Found nothing?!?! Either we hang forever, or we panic. */
> > + /*
> > + * Found nothing?!?! Either we hang forever, or we panic.
> > + * Do not panic when the OOM killer is sysrq triggered.
> > + */
>
> Again, it's clear what a conditional does in C code.
OK, I will remove it.
> > if (!p) {
> > - dump_header(NULL, gfp_mask, order, NULL, mpol_mask);
> > - panic("Out of memory and no killable processes...\n");
> > + if (!force_kill) {
> > + dump_header(NULL, gfp_mask, order, NULL, mpol_mask);
> > + panic("Out of memory and no killable processes...\n");
> > + } else {
> > + pr_info("Forced out of memory. No killable task found...\n");
> > + }
>
> This line could probably be reworded to specify that an oom kill was
> requested by a specific process and there was nothing avilable to kill.
> I'm not sure that "forced" implies that it was process triggered.
s@...ced@...rq triggered@ ?
>
> > }
> > if (p != (void *)-1UL) {
> > oom_kill_process(p, gfp_mask, order, points, totalpages, NULL,
---
diff --git a/Documentation/sysrq.txt b/Documentation/sysrq.txt
index a5dd88b0aede..7664e93411d2 100644
--- a/Documentation/sysrq.txt
+++ b/Documentation/sysrq.txt
@@ -76,7 +76,7 @@ On other - If you know of the key combos for other architectures, please
'e' - Send a SIGTERM to all processes, except for init.
'f' - Will call oom_kill to kill a memory hog process. Please note that
- an ongoing OOM killer is ignored and a task is killed even though
+ parallel OOM killer is ignored and a task is killed even though
there was an oom victim selected already. panic_on_oom is ignored
and the system doesn't panic if there are no oom killable tasks.
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 0c312eaac834..f2737d66f66a 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -687,7 +687,6 @@ bool out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
constraint = constrained_alloc(zonelist, gfp_mask, nodemask,
&totalpages);
mpol_mask = (constraint == CONSTRAINT_MEMORY_POLICY) ? nodemask : NULL;
- /* Ignore panic_on_oom when the OOM killer is sysrq triggered */
if (!force_kill)
check_panic_on_oom(constraint, gfp_mask, order, mpol_mask, NULL);
@@ -706,16 +705,13 @@ bool out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
}
p = select_bad_process(&points, totalpages, mpol_mask, force_kill);
- /*
- * Found nothing?!?! Either we hang forever, or we panic.
- * Do not panic when the OOM killer is sysrq triggered.
- */
+ /* Found nothing?!?! Either we hang forever, or we panic. */
if (!p) {
if (!force_kill) {
dump_header(NULL, gfp_mask, order, NULL, mpol_mask);
panic("Out of memory and no killable processes...\n");
} else {
- pr_info("Forced out of memory. No killable task found...\n");
+ pr_info("Sysrq triggered out of memory. No killable task found...\n");
}
}
if (p != (void *)-1UL) {
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists