Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1179764 > unrolled thread

[PATCH 4/4] oom: split out forced OOM killer

Started byMichal Hocko <mhocko@suse.com>
First post2015-07-08 15:10 +0200
Last post2015-07-09 01:50 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 4/4] oom: split out forced OOM killer Michal Hocko <mhocko@suse.com> - 2015-07-08 15:10 +0200
    Re: [PATCH 4/4] oom: split out forced OOM killer David Rientjes <rientjes@google.com> - 2015-07-09 01:50 +0200

#1179764 — [PATCH 4/4] oom: split out forced OOM killer

FromMichal Hocko <mhocko@suse.com>
Date2015-07-08 15:10 +0200
Subject[PATCH 4/4] oom: split out forced OOM killer
Message-ID<pJXgS-38d-19@gated-at.bofh.it>
From: Michal Hocko <mhocko@suse.cz>

The forced OOM killing is currently wired into out_of_memory() call
even though their objective is different which makes the code ugly
and harder to follow. Generic out_of_memory path has to deal with
configuration settings and heuristics which are completely irrelevant
to the forced OOM killer (e.g. sysctl_oom_kill_allocating_task or
OOM killer prevention for already dying tasks). All of them are
either relying on explicit force_kill check or indirectly by checking
current->mm which is always NULL for sysrq+f. This is not nice, hard
to follow and error prone.

Let's pull forced OOM killer code out into a separate function
(force_out_of_memory) which is really trivial now.
As a bonus we can clearly state that this is a forced OOM killer
in the OOM message which is helpful to distinguish it from the
regular OOM killer.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 drivers/tty/sysrq.c |  9 +--------
 include/linux/oom.h |  1 +
 mm/oom_kill.c       | 57 ++++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 865b837a9aee..6a3def693ded 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -356,15 +356,8 @@ static struct sysrq_key_op sysrq_term_op = {
 
 static void moom_callback(struct work_struct *ignored)
 {
-	const gfp_t gfp_mask = GFP_KERNEL;
-	struct oom_context oc = {
-		.zonelist = node_zonelist(first_memory_node, gfp_mask),
-		.gfp_mask = gfp_mask,
-		.force_kill = true,
-	};
-
 	mutex_lock(&oom_lock);
-	if (!out_of_memory(&oc))
+	if (!force_out_of_memory())
 		pr_info("OOM request ignored because killer is disabled\n");
 	mutex_unlock(&oom_lock);
 }
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 094407cb2d2e..6af2d12d6134 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -77,6 +77,7 @@ extern enum oom_scan_t oom_scan_process_thread(struct oom_context *oc,
 		struct task_struct *task, unsigned long totalpages);
 
 extern bool out_of_memory(struct oom_context *oc);
+extern bool force_out_of_memory(void);
 
 extern void exit_oom_victim(void);
 
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 01aa4cb86857..6a0b09296236 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -627,6 +627,38 @@ int unregister_oom_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL_GPL(unregister_oom_notifier);
 
 /**
+ * force_out_of_memory - forces OOM killer to kill a process
+ *
+ * Explicitly trigger the OOM killer. The system doesn't have to be under
+ * OOM condition (e.g. sysrq+f).
+ */
+bool force_out_of_memory(void)
+{
+	struct task_struct *p;
+	unsigned long totalpages;
+	unsigned int points;
+	const gfp_t gfp_mask = GFP_KERNEL;
+	struct oom_context oc = {
+		.zonelist = node_zonelist(first_memory_node, gfp_mask),
+		.gfp_mask = gfp_mask,
+		.force_kill = true,
+	};
+
+	if (oom_killer_disabled)
+		return false;
+
+	constrained_alloc(&oc, &totalpages);
+	p = select_bad_process(&oc, &points, totalpages);
+	if (p != (void *)-1UL)
+		oom_kill_process(&oc, p, points, totalpages, NULL,
+				 "Forced out of memory killer");
+	else
+		pr_warn("Sysrq triggered out of memory. No killable task found...\n");
+
+	return true;
+}
+
+/**
  * out_of_memory - kill the "best" process when we run out of memory
  * @oc: pointer to struct oom_context
  *
@@ -647,12 +679,10 @@ bool out_of_memory(struct oom_context *oc)
 	if (oom_killer_disabled)
 		return false;
 
-	if (!oc->force_kill) {
-		blocking_notifier_call_chain(&oom_notify_list, 0, &freed);
-		if (freed > 0)
-			/* Got some memory back in the last second. */
-			goto out;
-	}
+	blocking_notifier_call_chain(&oom_notify_list, 0, &freed);
+	if (freed > 0)
+		/* Got some memory back in the last second. */
+		goto out;
 
 	/*
 	 * If current has a pending SIGKILL or is exiting, then automatically
@@ -675,13 +705,8 @@ bool out_of_memory(struct oom_context *oc)
 	constraint = constrained_alloc(oc, &totalpages);
 	if (constraint != CONSTRAINT_MEMORY_POLICY)
 		oc->nodemask = NULL;
-	if (!oc->force_kill)
-		check_panic_on_oom(oc, constraint, NULL);
+	check_panic_on_oom(oc, constraint, NULL);
 
-	/*
-	 * not affecting force_kill because sysrq triggered OOM killer runs from
-	 * the workqueue context so current->mm will be NULL
-	 */
 	if (sysctl_oom_kill_allocating_task && current->mm &&
 	    !oom_unkillable_task(current, NULL, oc->nodemask) &&
 	    current->signal->oom_score_adj != OOM_SCORE_ADJ_MIN) {
@@ -694,12 +719,8 @@ bool out_of_memory(struct oom_context *oc)
 	p = select_bad_process(oc, &points, totalpages);
 	/* Found nothing?!?! Either we hang forever, or we panic. */
 	if (!p) {
-		if (!oc->force_kill) {
-			dump_header(oc, NULL, NULL);
-			panic("Out of memory and no killable processes...\n");
-		} else {
-			pr_info("Sysrq triggered out of memory. No killable task found...\n");
-		}
+		dump_header(oc, NULL, NULL);
+		panic("Out of memory and no killable processes...\n");
 	}
 	if (p != (void *)-1UL) {
 		oom_kill_process(oc, p, points, totalpages, NULL,
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1180304

FromDavid Rientjes <rientjes@google.com>
Date2015-07-09 01:50 +0200
Message-ID<pK7gd-183-1@gated-at.bofh.it>
In reply to#1179764
On Wed, 8 Jul 2015, Michal Hocko wrote:

> From: Michal Hocko <mhocko@suse.cz>
> 
> The forced OOM killing is currently wired into out_of_memory() call
> even though their objective is different which makes the code ugly
> and harder to follow. Generic out_of_memory path has to deal with
> configuration settings and heuristics which are completely irrelevant
> to the forced OOM killer (e.g. sysctl_oom_kill_allocating_task or
> OOM killer prevention for already dying tasks). All of them are
> either relying on explicit force_kill check or indirectly by checking
> current->mm which is always NULL for sysrq+f. This is not nice, hard
> to follow and error prone.
> 
> Let's pull forced OOM killer code out into a separate function
> (force_out_of_memory) which is really trivial now.
> As a bonus we can clearly state that this is a forced OOM killer
> in the OOM message which is helpful to distinguish it from the
> regular OOM killer.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.cz>

It's really absurd that we have to go through this over and over and that 
your patches are actually being merged into -mm just because you don't get 
the point.

We have no need for a force_out_of_memory() function.  None whatsoever.  
Keeping oc->force_kill around is just more pointless space on a very deep 
stack and I'm tired of fixing stack overflows.  I'm certainly not going to 
introduce others because you think it looks cleaner in the code when 
memory compaction does the exact same thing by using cc->order == -1 to 
mean explicit compaction.

This is turning into a complete waste of time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web