[patch 0/8] oom killer updates

[patch 0/8] oom killer updates

[David Rientjes]

This patchset is an updated replacement of all patches posted by me to
linux-mm on September 18, 2007.
---
 Documentation/cpusets.txt |    6 +-
 drivers/char/sysrq.c      |    1 +
 include/linux/cpuset.h    |   13 ++--
 include/linux/oom.h       |   23 +++++-
 include/linux/swap.h      |    5 -
 kernel/cpuset.c           |   61 ++++++++-------
 mm/oom_kill.c             |  197 ++++++++++++++++++++++++++++++++++++---------
 mm/page_alloc.c           |   15 +++-
 8 files changed, 240 insertions(+), 81 deletions(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 1/8] oom: move prototypes to appropriate header file

[David Rientjes]

Move the OOM killer's extern function prototypes to include/linux/oom.h
and include it where necessary.

Cc: Andrea Arcangeli <andrea@suse.de>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 drivers/char/sysrq.c |    1 +
 include/linux/oom.h  |   11 ++++++++++-
 include/linux/swap.h |    5 -----
 mm/page_alloc.c      |    1 +
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -36,6 +36,7 @@
 #include <linux/kexec.h>
 #include <linux/irq.h>
 #include <linux/hrtimer.h>
+#include <linux/oom.h>
 
 #include <asm/ptrace.h>
 #include <asm/irq_regs.h>
diff --git a/include/linux/oom.h b/include/linux/oom.h
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -1,10 +1,19 @@
 #ifndef __INCLUDE_LINUX_OOM_H
 #define __INCLUDE_LINUX_OOM_H
 
+#include <linux/sched.h>
+
 /* /proc/<pid>/oom_adj set to -17 protects from the oom-killer */
 #define OOM_DISABLE (-17)
 /* inclusive */
 #define OOM_ADJUST_MIN (-16)
 #define OOM_ADJUST_MAX 15
 
-#endif
+#ifdef __KERNEL__
+
+extern void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order);
+extern int register_oom_notifier(struct notifier_block *nb);
+extern int unregister_oom_notifier(struct notifier_block *nb);
+
+#endif /* __KERNEL__*/
+#endif /* _INCLUDE_LINUX_OOM_H */
diff --git a/include/linux/swap.h b/include/linux/swap.h
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -158,11 +158,6 @@ struct swap_list_t {
 /* Swap 50% full? Release swapcache more aggressively.. */
 #define vm_swap_full() (nr_swap_pages*2 < total_swap_pages)
 
-/* linux/mm/oom_kill.c */
-extern void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order);
-extern int register_oom_notifier(struct notifier_block *nb);
-extern int unregister_oom_notifier(struct notifier_block *nb);
-
 /* linux/mm/memory.c */
 extern void swapin_readahead(swp_entry_t, unsigned long, struct vm_area_struct *);
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -41,6 +41,7 @@
 #include <linux/pfn.h>
 #include <linux/backing-dev.h>
 #include <linux/fault-inject.h>
+#include <linux/oom.h>
 
 #include <asm/tlbflush.h>
 #include <asm/div64.h>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 2/8] oom: move constraints to enum

[David Rientjes]

The OOM killer's CONSTRAINT definitions are really more appropriate in an
enum, so define them in include/linux/oom.h.

Cc: Andrea Arcangeli <andrea@suse.de>
Acked-by: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 include/linux/oom.h |    9 +++++++++
 mm/oom_kill.c       |   12 +++---------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/linux/oom.h b/include/linux/oom.h
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -11,6 +11,15 @@
 
 #ifdef __KERNEL__
 
+/*
+ * Types of limitations to the nodes from which allocations may occur
+ */
+enum oom_constraint {
+	CONSTRAINT_NONE,
+	CONSTRAINT_CPUSET,
+	CONSTRAINT_MEMORY_POLICY,
+};
+
 extern void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order);
 extern int register_oom_notifier(struct notifier_block *nb);
 extern int unregister_oom_notifier(struct notifier_block *nb);
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -164,16 +164,10 @@ unsigned long badness(struct task_struct *p, unsigned long uptime)
 }
 
 /*
- * Types of limitations to the nodes from which allocations may occur
- */
-#define CONSTRAINT_NONE 1
-#define CONSTRAINT_MEMORY_POLICY 2
-#define CONSTRAINT_CPUSET 3
-
-/*
  * Determine the type of allocation constraint.
  */
-static inline int constrained_alloc(struct zonelist *zonelist, gfp_t gfp_mask)
+static inline enum oom_constraint constrained_alloc(struct zonelist *zonelist,
+						    gfp_t gfp_mask)
 {
 #ifdef CONFIG_NUMA
 	struct zone **z;
@@ -400,7 +394,7 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order)
 	struct task_struct *p;
 	unsigned long points = 0;
 	unsigned long freed = 0;
-	int constraint;
+	enum oom_constraint constraint;
 
 	blocking_notifier_call_chain(&oom_notify_list, 0, &freed);
 	if (freed > 0)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 3/8] oom: save zonelist pointer for oom killer calls

[David Rientjes]

OOM killer synchronization should be done with zone granularity so that
memory policy and cpuset allocations may have their corresponding zones
locked and allow parallel kills for other OOM conditions that may exist
elsewhere in the system.  DMA allocations can be targeted at the zone
level, which would not be possible if locking was done in nodes or
globally.

A pointer to the OOM-triggering zonelist is saved in a linked list.  Any
time there is an OOM condition, all zones in the zonelist are checked
against the zonelists stored in the OOM killer lists.  If the OOM killer
has already been called for an allocation that includes one of these
zones, the "trylock" fails and returns zero.

The OOM killer is only invoked in low memory situations, so it is helpful
to enable to OOM-triggering task with PF_MEMALLOC so it can allocate
without watermarks.  This task will enable future memory freeing, so
setting PF_MEMALLOC is entirely acceptable.

If the kzalloc() still does not succeed, the return value of
try_set_zone_oom() is non-zero which allows the OOM killer to proceed
without saving the zonelist.  kfree(NULL) is acceptable in this case
in clear_zonelist_oom().

Cc: Andrea Arcangeli <andrea@suse.de>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 include/linux/oom.h |    3 ++
 mm/oom_kill.c       |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/include/linux/oom.h b/include/linux/oom.h
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -20,6 +20,9 @@ enum oom_constraint {
 	CONSTRAINT_MEMORY_POLICY,
 };
 
+extern int try_set_zone_oom(struct zonelist *zonelist);
+extern void clear_zonelist_oom(const struct zonelist *zonelist);
+
 extern void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order);
 extern int register_oom_notifier(struct notifier_block *nb);
 extern int unregister_oom_notifier(struct notifier_block *nb);
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -26,6 +26,13 @@
 #include <linux/module.h>
 #include <linux/notifier.h>
 
+struct oom_zonelist {
+	struct zonelist *zonelist;
+	struct list_head list;
+};
+static LIST_HEAD(zonelists);
+static DEFINE_MUTEX(oom_zonelist_mutex);
+
 int sysctl_panic_on_oom;
 /* #define DEBUG */
 
@@ -381,6 +388,75 @@ int unregister_oom_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(unregister_oom_notifier);
 
+/*
+ * Call with oom_zonelist_mutex held.
+ */
+static int is_zone_locked(const struct zone *zone)
+{
+	struct oom_zonelist *oom_zl;
+	int i;
+
+	list_for_each_entry(oom_zl, &zonelists, list)
+		for (i = 0; oom_zl->zonelist->zones[i]; i++)
+			if (zone == oom_zl->zonelist->zones[i])
+				return 1;
+	return 0;
+}
+
+/*
+ * Try to acquire the OOM killer lock for the zones in zonelist.  Returns zero
+ * if a parallel OOM killing is already taking place that includes a zone in
+ * the zonelist.
+ */
+int try_set_zone_oom(struct zonelist *zonelist)
+{
+	struct oom_zonelist *oom_zl;
+	int ret = 1;
+	int i;
+
+	mutex_lock(&oom_zonelist_mutex);
+	for (i = 0; zonelist->zones[i]; i++)
+		if (is_zone_locked(zonelist->zones[i])) {
+			ret = 0;
+			goto out;
+		}
+
+	/*
+	 * PF_MEMALLOC is used for tasks that will enable future memory
+	 * freeing, so it is appropriate in this case where memory is
+	 * low.
+	 */
+	current->flags |= PF_MEMALLOC;
+	oom_zl = kzalloc(sizeof(*oom_zl), GFP_KERNEL);
+	current->flags &= ~PF_MEMALLOC;
+	if (!oom_zl)
+		goto out;
+
+	oom_zl->zonelist = zonelist;
+	list_add(&oom_zl->list, &zonelists);
+out:
+	mutex_unlock(&oom_zonelist_mutex);
+	return ret;
+}
+
+/*
+ * Removes the zonelist from the list so that future allocations that include
+ * its zones can successfully call the OOM killer.
+ */
+void clear_zonelist_oom(const struct zonelist *zonelist)
+{
+	struct oom_zonelist *oom_zl = NULL;
+
+	mutex_lock(&oom_zonelist_mutex);
+	list_for_each_entry(oom_zl, &zonelists, list)
+		if (zonelist == oom_zl->zonelist) {
+			list_del(&oom_zl->list);
+			break;
+		}
+	mutex_unlock(&oom_zonelist_mutex);
+	kfree(oom_zl);
+}
+
 /**
  * out_of_memory - kill the "best" process when we run out of memory
  *

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 4/8] oom: serialize out of memory calls

[David Rientjes]

Before invoking the OOM killer, a final allocation attempt with a very
high watermark is attempted.  Serialization needs to occur at this point
or it may be possible that the allocation could succeed after acquiring
the lock.  If the lock is contended, the task is put to sleep and the
allocation attempt is retried when rescheduled.

Cc: Andrea Arcangeli <andrea@suse.de>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/page_alloc.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1353,6 +1353,11 @@ nofail_alloc:
 		if (page)
 			goto got_pg;
 	} else if ((gfp_mask & __GFP_FS) && !(gfp_mask & __GFP_NORETRY)) {
+		if (!try_set_zone_oom(zonelist)) {
+			schedule_timeout_uninterruptible(1);
+			goto restart;
+		}
+
 		/*
 		 * Go through the zonelist yet one more time, keep
 		 * very high watermark here, this is only to catch
@@ -1361,14 +1366,19 @@ nofail_alloc:
 		 */
 		page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, order,
 				zonelist, ALLOC_WMARK_HIGH|ALLOC_CPUSET);
-		if (page)
+		if (page) {
+			clear_zonelist_oom(zonelist);
 			goto got_pg;
+		}
 
 		/* The OOM killer will not help higher order allocs so fail */
-		if (order > PAGE_ALLOC_COSTLY_ORDER)
+		if (order > PAGE_ALLOC_COSTLY_ORDER) {
+			clear_zonelist_oom(zonelist);
 			goto nopage;
+		}
 
 		out_of_memory(zonelist, gfp_mask, order);
+		clear_zonelist_oom(zonelist);
 		goto restart;
 	}
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 5/8] oom: add per-cpuset file oom_kill_asking_task

[David Rientjes]

Adds a per-cpuset file 'oom_kill_asking_task', which by default is set to
zero.  If enabled, current is always killed whenever a cpuset-constrained
OOM is triggered; otherwise, the tasklist is scanned for a memory-
hogging task to kill.  Those tasks that do not share exclusive memory
nodes with current are penalized by eight times in the badness scoring.

The value of 'oom_kill_asking_task' is inherited from parent cpusets.

Cc: Andrea Arcangeli <andrea@suse.de>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 Documentation/cpusets.txt |    6 +++++-
 include/linux/cpuset.h    |    7 +++++++
 kernel/cpuset.c           |   34 +++++++++++++++++++++++++++++++++-
 mm/oom_kill.c             |   10 ++++++++++
 4 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/Documentation/cpusets.txt b/Documentation/cpusets.txt
--- a/Documentation/cpusets.txt
+++ b/Documentation/cpusets.txt
@@ -181,6 +181,9 @@ containing the following files describing that cpuset:
  - tasks: list of tasks (by pid) attached to that cpuset
  - notify_on_release flag: run /sbin/cpuset_release_agent on exit?
  - memory_pressure: measure of how much paging pressure in cpuset
+ - oom_kill_asking_task: in OOM situations, always kill the task that
+	triggered the condition and avoid scanning the tasklist to find
+	the ideal target
 
 In addition, the root cpuset only has the following file:
  - memory_pressure_enabled flag: compute memory_pressure?
@@ -351,7 +354,8 @@ except perhaps as modified by the tasks NUMA mempolicy or cpuset
 configuration, so long as sufficient free memory pages are available.
 
 When new cpusets are created, they inherit the memory spread settings
-of their parent.
+and OOM killer behavior as specified by oom_kill_asking_task of their
+parent.
 
 Setting memory spreading causes allocations for the affected page
 or slab caches to ignore the tasks NUMA mempolicy and be spread
diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -75,6 +75,8 @@ static inline int cpuset_do_slab_mem_spread(void)
 
 extern void cpuset_track_online_nodes(void);
 
+extern int oom_kill_asking_task(const struct task_struct *task);
+
 #else /* !CONFIG_CPUSETS */
 
 static inline int cpuset_init_early(void) { return 0; }
@@ -146,6 +148,11 @@ static inline int cpuset_do_slab_mem_spread(void)
 
 static inline void cpuset_track_online_nodes(void) {}
 
+static inline int oom_kill_asking_task(const struct task_struct *task)
+{
+	return 1;
+}
+
 #endif /* !CONFIG_CPUSETS */
 
 #endif /* _LINUX_CPUSET_H */
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -107,6 +107,7 @@ typedef enum {
 	CS_MEMORY_MIGRATE,
 	CS_REMOVED,
 	CS_NOTIFY_ON_RELEASE,
+	CS_OOM_KILL_ASKING_TASK,
 	CS_SPREAD_PAGE,
 	CS_SPREAD_SLAB,
 } cpuset_flagbits_t;
@@ -132,6 +133,11 @@ static inline int notify_on_release(const struct cpuset *cs)
 	return test_bit(CS_NOTIFY_ON_RELEASE, &cs->flags);
 }
 
+static inline int is_oom_kill_asking_task(const struct cpuset *cs)
+{
+	return test_bit(CS_OOM_KILL_ASKING_TASK, &cs->flags);
+}
+
 static inline int is_memory_migrate(const struct cpuset *cs)
 {
 	return test_bit(CS_MEMORY_MIGRATE, &cs->flags);
@@ -1056,7 +1062,8 @@ static int update_memory_pressure_enabled(struct cpuset *cs, char *buf)
  * update_flag - read a 0 or a 1 in a file and update associated flag
  * bit:	the bit to update (CS_CPU_EXCLUSIVE, CS_MEM_EXCLUSIVE,
  *				CS_NOTIFY_ON_RELEASE, CS_MEMORY_MIGRATE,
- *				CS_SPREAD_PAGE, CS_SPREAD_SLAB)
+ *				CS_OOM_KILL_ASKING_TASK, CS_SPREAD_PAGE,
+ *				CS_SPREAD_SLAB)
  * cs:	the cpuset to update
  * buf:	the buffer where we read the 0 or 1
  *
@@ -1299,6 +1306,7 @@ typedef enum {
 	FILE_SPREAD_PAGE,
 	FILE_SPREAD_SLAB,
 	FILE_TASKLIST,
+	FILE_OOM_KILL_ASKING_TASK,
 } cpuset_filetype_t;
 
 static ssize_t cpuset_common_file_write(struct file *file,
@@ -1369,6 +1377,9 @@ static ssize_t cpuset_common_file_write(struct file *file,
 	case FILE_TASKLIST:
 		retval = attach_task(cs, buffer, &pathbuf);
 		break;
+	case FILE_OOM_KILL_ASKING_TASK:
+		retval = update_flag(CS_OOM_KILL_ASKING_TASK, cs, buffer);
+		break;
 	default:
 		retval = -EINVAL;
 		goto out2;
@@ -1481,6 +1492,9 @@ static ssize_t cpuset_common_file_read(struct file *file, char __user *buf,
 	case FILE_SPREAD_SLAB:
 		*s++ = is_spread_slab(cs) ? '1' : '0';
 		break;
+	case FILE_OOM_KILL_ASKING_TASK:
+		*s++ = is_oom_kill_asking_task(cs) ? '1' : '0';
+		break;
 	default:
 		retval = -EINVAL;
 		goto out;
@@ -1849,6 +1863,11 @@ static struct cftype cft_spread_slab = {
 	.private = FILE_SPREAD_SLAB,
 };
 
+static struct cftype cft_oom_kill_asking_task = {
+	.name = "oom_kill_asking_task",
+	.private = FILE_OOM_KILL_ASKING_TASK,
+};
+
 static int cpuset_populate_dir(struct dentry *cs_dentry)
 {
 	int err;
@@ -1873,6 +1892,8 @@ static int cpuset_populate_dir(struct dentry *cs_dentry)
 		return err;
 	if ((err = cpuset_add_file(cs_dentry, &cft_tasks)) < 0)
 		return err;
+	if ((err = cpuset_add_file(cs_dentry, &cft_oom_kill_asking_task)) < 0)
+		return err;
 	return 0;
 }
 
@@ -1903,6 +1924,8 @@ static long cpuset_create(struct cpuset *parent, const char *name, int mode)
 		set_bit(CS_SPREAD_PAGE, &cs->flags);
 	if (is_spread_slab(parent))
 		set_bit(CS_SPREAD_SLAB, &cs->flags);
+	if (is_oom_kill_asking_task(parent))
+		set_bit(CS_OOM_KILL_ASKING_TASK, &cs->flags);
 	cs->cpus_allowed = CPU_MASK_NONE;
 	cs->mems_allowed = NODE_MASK_NONE;
 	atomic_set(&cs->count, 0);
@@ -2565,6 +2588,15 @@ int cpuset_mem_spread_node(void)
 }
 EXPORT_SYMBOL_GPL(cpuset_mem_spread_node);
 
+/*
+ * Returns non-zero if 'oom_kill_asking_task' is set for this cpuset; otherwise
+ * returns zero.
+ */
+int oom_kill_asking_task(const struct task_struct *task)
+{
+	return is_oom_kill_asking_task(task->cpuset);
+}
+
 /**
  * cpuset_excl_nodes_overlap - Do we overlap @p's mem_exclusive ancestors?
  * @p: pointer to task_struct of some other task.
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -496,6 +496,16 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order)
 		break;
 
 	case CONSTRAINT_CPUSET:
+		/*
+		 * If the cpuset's "oom_kill_asking_task" flag is not set, the
+		 * OOM killer uses the same heuristics as a non-constrained
+		 * allocation attempt to kill a memory-hogging task.  The
+		 * badness score favors killing tasks that share exclusive
+		 * mems with current.
+		 */
+		if (!oom_kill_asking_task(current))
+			goto retry;
+
 		oom_kill_process(current, points,
 				"No available memory in cpuset");
 		break;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 6/8] oom: suppress extraneous stack and memory dump

[David Rientjes]

Suppresses the extraneous stack and memory dump when a parallel OOM
killing has been found.  There's no need to fill the ring buffer with
this information if its already been printed and the condition that
triggered the previous OOM killer has not yet been alleviated.

Cc: Andrea Arcangeli <andrea@suse.de>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/oom_kill.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -345,12 +345,20 @@ static int oom_kill_task(struct task_struct *p)
 	return 0;
 }
 
-static int oom_kill_process(struct task_struct *p, unsigned long points,
-		const char *message)
+static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
+			    unsigned long points, const char *message)
 {
 	struct task_struct *c;
 	struct list_head *tsk;
 
+	if (printk_ratelimit()) {
+		printk(KERN_WARNING "%s invoked oom-killer: "
+			"gfp_mask=0x%x, order=%d, oomkilladj=%d\n",
+			current->comm, gfp_mask, order, current->oomkilladj);
+		dump_stack();
+		show_mem();
+	}
+
 	/*
 	 * If the task is already exiting, don't alarm the sysadmin or kill
 	 * its children or threads, just set TIF_MEMDIE so it can die quickly
@@ -477,14 +485,6 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order)
 		/* Got some memory back in the last second. */
 		return;
 
-	if (printk_ratelimit()) {
-		printk(KERN_WARNING "%s invoked oom-killer: "
-			"gfp_mask=0x%x, order=%d, oomkilladj=%d\n",
-			current->comm, gfp_mask, order, current->oomkilladj);
-		dump_stack();
-		show_mem();
-	}
-
 	if (sysctl_panic_on_oom == 2)
 		panic("out of memory. Compulsory panic_on_oom is selected.\n");
 
@@ -498,7 +498,7 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order)
 
 	switch (constraint) {
 	case CONSTRAINT_MEMORY_POLICY:
-		oom_kill_process(current, points,
+		oom_kill_process(current, gfp_mask, order, points,
 				"No available memory (MPOL_BIND)");
 		break;
 
@@ -513,7 +513,7 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order)
 		if (!oom_kill_asking_task(current))
 			goto retry;
 
-		oom_kill_process(current, points,
+		oom_kill_process(current, gfp_mask, order, points,
 				"No available memory in cpuset");
 		break;
 
@@ -537,7 +537,8 @@ retry:
 			panic("Out of memory and no killable processes...\n");
 		}
 
-		if (oom_kill_process(p, points, "Out of memory"))
+		if (oom_kill_process(p, gfp_mask, order, points,
+				     "Out of memory"))
 			goto retry;
 
 		break;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 7/8] oom: only kill tasks that share zones with zonelist

[David Rientjes]

It is only helpful to OOM kill tasks that share a zone with at least one
zone in the zonelist passed to __alloc_pages().  The current task, of
course, always shares zones with this zonelist so it will always be
selected if it is killable and no other task can be found.  Otherwise, we
panic.

To determine whether a candidate task shares a zone with a member of the
zonelist, it is necessary to iterate through the VMA's of each task.
This isn't as painful as it first appears since usually a VMA's zone
will intersect with the OOM-triggering zonelist early in the scan; in
that case, it is not necessary to continue scanning for that task.

mm->mmap_sem is required to read scan through the VMA's but it may not
immediately be available because it is write-locked.  Instead of
sleeping on the semaphore, we simply score the task using the normal
heuristics and assume it will be beneficial to kill the task.  We are
not assured that the write-lock will ever be released in an OOM
condition and mmap_sem is frequently contended.

Cc: Andrea Arcangeli <andrea@suse.de>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/oom_kill.c |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -25,6 +25,7 @@
 #include <linux/cpuset.h>
 #include <linux/module.h>
 #include <linux/notifier.h>
+#include <linux/pfn.h>
 
 struct oom_zonelist {
 	struct zonelist *zonelist;
@@ -201,30 +202,80 @@ static inline enum oom_constraint constrained_alloc(struct zonelist *zonelist,
 }
 
 /*
+ * Returns non-zero if zone is found in zonelist; otherwise, returns zero.
+ */
+static int is_zone_in_zonelist(struct zone *zone, struct zonelist *zonelist)
+{
+	struct zone **z;
+
+	z = zonelist->zones;
+	do {
+		if (zone == *z)
+			return 1;
+	} while (*(++z) != NULL);
+	return 0;
+}
+
+/*
  * Simple selection loop. We chose the process with the highest
  * number of 'points'. We expect the caller will lock the tasklist.
  *
  * (not docbooked, we don't want this one cluttering up the manual)
  */
-static struct task_struct *select_bad_process(unsigned long *ppoints)
+static struct task_struct *select_bad_process(struct zonelist *zonelist,
+				gfp_t gfp_mask, unsigned long *ppoints)
 {
 	struct task_struct *g, *p;
 	struct task_struct *chosen = NULL;
+	struct mm_struct *mm;
+	struct vm_area_struct *vma;
 	struct timespec uptime;
 	*ppoints = 0;
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	do_each_thread(g, p) {
 		unsigned long points;
+		int has_zone = 0;
 
+		/* skip the init task */
+		if (is_init(p))
+			continue;
 		/*
 		 * skip kernel threads and tasks which have already released
 		 * their mm.
 		 */
-		if (!p->mm)
+		mm = get_task_mm(p);
+		if (!mm)
 			continue;
-		/* skip the init task */
-		if (is_init(p))
+
+		/*
+		 * If mm->mmap_sem is write-locked, the task is assumed to be
+		 * a worthwhile target.  It may not be possible for the task
+		 * to ever unlock.
+		 */
+		if (!down_read_trylock(&mm->mmap_sem))
+			goto no_zonescan;
+		for (vma = mm->mmap; vma; vma = vma->vm_next) {
+			unsigned long pfn;
+			struct zone *zone;
+
+			pfn = PFN_DOWN(vma->vm_start);
+			zone = page_zone(pfn_to_page(pfn));
+
+			if (is_zone_in_zonelist(zone, zonelist)) {
+				has_zone = 1;
+				break;
+			}
+		}
+		up_read(&mm->mmap_sem);
+no_zonescan:
+		mmput(mm);
+
+		/*
+		 * It will not do any good to kill p if it does not share any
+		 * zones with the zonelist in our allocation attempt.
+		 */
+		if (!has_zone)
 			continue;
 
 		/*
@@ -525,7 +576,7 @@ retry:
 		 * Rambo mode: Shoot down a process and hope it solves whatever
 		 * issues we may have.
 		 */
-		p = select_bad_process(&points);
+		p = select_bad_process(zonelist, gfp_mask, &points);
 
 		if (PTR_ERR(p) == -1UL)
 			goto out;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 8/8] oom: do not check cpuset in badness scoring

[David Rientjes]

It is no longer necessary to check whether a task's cpuset nodes overlap
with current because the tasklist has already been filtered with respect
to zones shared in the zonelist.

This leads to a nice optimization since callback_mutex is no longer
required; we can now have parallel OOM killings happening in separate
cpusets if their memory is mutually exclusive.

Cc: Andrea Arcangeli <andrea@suse.de>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 include/linux/cpuset.h |    6 ------
 kernel/cpuset.c        |   27 ---------------------------
 mm/oom_kill.c          |   11 -----------
 3 files changed, 0 insertions(+), 44 deletions(-)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -58,9 +58,6 @@ extern void __cpuset_memory_pressure_bump(void);
 extern const struct file_operations proc_cpuset_operations;
 extern char *cpuset_task_status_allowed(struct task_struct *task, char *buffer);
 
-extern void cpuset_lock(void);
-extern void cpuset_unlock(void);
-
 extern int cpuset_mem_spread_node(void);
 
 static inline int cpuset_do_page_mem_spread(void)
@@ -128,9 +125,6 @@ static inline char *cpuset_task_status_allowed(struct task_struct *task,
 	return buffer;
 }
 
-static inline void cpuset_lock(void) {}
-static inline void cpuset_unlock(void) {}
-
 static inline int cpuset_mem_spread_node(void)
 {
 	return 0;
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2524,33 +2524,6 @@ int __cpuset_zone_allowed_hardwall(struct zone *z, gfp_t gfp_mask)
 }
 
 /**
- * cpuset_lock - lock out any changes to cpuset structures
- *
- * The out of memory (oom) code needs to mutex_lock cpusets
- * from being changed while it scans the tasklist looking for a
- * task in an overlapping cpuset.  Expose callback_mutex via this
- * cpuset_lock() routine, so the oom code can lock it, before
- * locking the task list.  The tasklist_lock is a spinlock, so
- * must be taken inside callback_mutex.
- */
-
-void cpuset_lock(void)
-{
-	mutex_lock(&callback_mutex);
-}
-
-/**
- * cpuset_unlock - release lock on cpuset changes
- *
- * Undo the lock taken in a previous cpuset_lock() call.
- */
-
-void cpuset_unlock(void)
-{
-	mutex_unlock(&callback_mutex);
-}
-
-/**
  * cpuset_mem_spread_node() - On which node to begin search for a page
  *
  * If a task is marked PF_SPREAD_PAGE or PF_SPREAD_SLAB (as for
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -145,14 +145,6 @@ unsigned long badness(struct task_struct *p, unsigned long uptime)
 		points /= 4;
 
 	/*
-	 * If p's nodes don't overlap ours, it may still help to kill p
-	 * because p may have allocated or otherwise mapped memory on
-	 * this node before. However it will be less likely.
-	 */
-	if (!cpuset_excl_nodes_overlap(p))
-		points /= 8;
-
-	/*
 	 * Adjust the score by oomkilladj.
 	 */
 	if (p->oomkilladj) {
@@ -532,7 +524,6 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order)
 	 * NUMA) that may require different handling.
 	 */
 	constraint = constrained_alloc(zonelist, gfp_mask);
-	cpuset_lock();
 	read_lock(&tasklist_lock);
 
 	switch (constraint) {
@@ -575,7 +566,6 @@ retry:
 		/* Found nothing?!?! Either we hang forever, or we panic. */
 		if (!p) {
 			read_unlock(&tasklist_lock);
-			cpuset_unlock();
 			panic("Out of memory and no killable processes...\n");
 		}
 
@@ -587,7 +577,6 @@ retry:
 
 out:
 	read_unlock(&tasklist_lock);
-	cpuset_unlock();
 
 	/*
 	 * Give "p" a good chance of killing itself before we

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 8/8] oom: do not check cpuset in badness scoring

[Christoph Lameter]

On Wed, 19 Sep 2007, David Rientjes wrote:

> It is no longer necessary to check whether a task's cpuset nodes overlap
> with current because the tasklist has already been filtered with respect
> to zones shared in the zonelist.

I doubt it. You would have to scan over all pages mapped by a process and 
build zonelists to check that.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 7/8] oom: only kill tasks that share zones with zonelist

[Christoph Lameter]

On Wed, 19 Sep 2007, David Rientjes wrote:

> +		for (vma = mm->mmap; vma; vma = vma->vm_next) {
> +			unsigned long pfn;
> +			struct zone *zone;
> +
> +			pfn = PFN_DOWN(vma->vm_start);
> +			zone = page_zone(pfn_to_page(pfn));

This seems to assume that all pages in a vma are in the same zone? That is 
not the case. On a NUMA system pages may be allocated round robin. Meaning 
lots of zones are used that this approach does not catch.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 7/8] oom: only kill tasks that share zones with zonelist

[David Rientjes]

On Wed, 19 Sep 2007, Christoph Lameter wrote:

> > +		for (vma = mm->mmap; vma; vma = vma->vm_next) {
> > +			unsigned long pfn;
> > +			struct zone *zone;
> > +
> > +			pfn = PFN_DOWN(vma->vm_start);
> > +			zone = page_zone(pfn_to_page(pfn));
> 
> This seems to assume that all pages in a vma are in the same zone? That is 
> not the case. On a NUMA system pages may be allocated round robin. Meaning 
> lots of zones are used that this approach does not catch.
> 

Setting the CONSTRAINT_MEMORY_POLICY case aside for a moment, what stops 
us from getting rid of taking callback_mutex and simply relying on the 
following to filter for candidate tasks:

	do_each_thread(g, p) {
		...
		/*
		 * Check if it will do any good to kill this task based
		 * on where it is allowed to allocate.
		 */
		if (!nodes_intersects(current->mems_allowed,
				      p->mems_allowed))
			continue;
		...
	} while_each_thread(g, p);

We shouldn't really be concerned with the changing cpuset states during 
out_of_memory() since we're only using it as a hint and we're not 
dereferencing current->cpuset or p->cpuset.  This eliminates the need for 
cpuset_{lock,unlock}() and cpuset_excl_nodes_overlap().

		David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 7/8] oom: only kill tasks that share zones with zonelist

[Christoph Lameter]

On Wed, 19 Sep 2007, David Rientjes wrote:

> > This seems to assume that all pages in a vma are in the same zone? That is 
> > not the case. On a NUMA system pages may be allocated round robin. Meaning 
> > lots of zones are used that this approach does not catch.
> > 
> 
> Setting the CONSTRAINT_MEMORY_POLICY case aside for a moment, what stops 
> us from getting rid of taking callback_mutex and simply relying on the 
> following to filter for candidate tasks:
> 
> 	do_each_thread(g, p) {
> 		...
> 		/*
> 		 * Check if it will do any good to kill this task based
> 		 * on where it is allowed to allocate.
> 		 */
> 		if (!nodes_intersects(current->mems_allowed,
> 				      p->mems_allowed))
> 			continue;
> 		...
> 	} while_each_thread(g, p);

A global scan over all processes is expensive and may take a long time if 
you have a 100000 or so of them.

> We shouldn't really be concerned with the changing cpuset states during 
> out_of_memory() since we're only using it as a hint and we're not 
> dereferencing current->cpuset or p->cpuset.  This eliminates the need for 
> cpuset_{lock,unlock}() and cpuset_excl_nodes_overlap().

Yup.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 7/8] oom: only kill tasks that share zones with zonelist

[David Rientjes]

On Thu, 20 Sep 2007, Christoph Lameter wrote:

> > Setting the CONSTRAINT_MEMORY_POLICY case aside for a moment, what stops 
> > us from getting rid of taking callback_mutex and simply relying on the 
> > following to filter for candidate tasks:
> > 
> > 	do_each_thread(g, p) {
> > 		...
> > 		/*
> > 		 * Check if it will do any good to kill this task based
> > 		 * on where it is allowed to allocate.
> > 		 */
> > 		if (!nodes_intersects(current->mems_allowed,
> > 				      p->mems_allowed))
> > 			continue;
> > 		...
> > 	} while_each_thread(g, p);
> 
> A global scan over all processes is expensive and may take a long time if 
> you have a 100000 or so of them.
> 

Yeah, I understand that.  Paul and I talked about it a while ago and 
decided that a per-cpuset file 'oom_kill_asking_task' could be implemented 
to determine whether the OOM killer would simply kill current or go 
through select_bad_process() in the CONSTRAINT_CPUSET case to address that 
problem.  Let me know if that doesn't seem good enough.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 7/8] oom: only kill tasks that share zones with zonelist

[Christoph Lameter]

On Thu, 20 Sep 2007, David Rientjes wrote:

> On Thu, 20 Sep 2007, Christoph Lameter wrote:
> 
> > > Setting the CONSTRAINT_MEMORY_POLICY case aside for a moment, what stops 
> > > us from getting rid of taking callback_mutex and simply relying on the 
> > > following to filter for candidate tasks:
> > > 
> > > 	do_each_thread(g, p) {
> > > 		...
> > > 		/*
> > > 		 * Check if it will do any good to kill this task based
> > > 		 * on where it is allowed to allocate.
> > > 		 */
> > > 		if (!nodes_intersects(current->mems_allowed,
> > > 				      p->mems_allowed))
> > > 			continue;
> > > 		...
> > > 	} while_each_thread(g, p);
> > 
> > A global scan over all processes is expensive and may take a long time if 
> > you have a 100000 or so of them.
> > 
> 
> Yeah, I understand that.  Paul and I talked about it a while ago and 
> decided that a per-cpuset file 'oom_kill_asking_task' could be implemented 
> to determine whether the OOM killer would simply kill current or go 
> through select_bad_process() in the CONSTRAINT_CPUSET case to address that 
> problem.  Let me know if that doesn't seem good enough.

There are still allocations that are not constrained to a cpuset. If the 
has an OOM condition on an allocation that is not constrained then the 
above may still cause more troubles.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 4/8] oom: serialize out of memory calls

[Christoph Lameter]

On Wed, 19 Sep 2007, David Rientjes wrote:

> Before invoking the OOM killer, a final allocation attempt with a very
> high watermark is attempted.  Serialization needs to occur at this point
> or it may be possible that the allocation could succeed after acquiring
> the lock.  If the lock is contended, the task is put to sleep and the
> allocation attempt is retried when rescheduled.

The problem with a succeeding allocation is that it takes memory 
away from the OOM killer?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 4/8] oom: serialize out of memory calls

[David Rientjes]

On Wed, 19 Sep 2007, Christoph Lameter wrote:

> > Before invoking the OOM killer, a final allocation attempt with a very
> > high watermark is attempted.  Serialization needs to occur at this point
> > or it may be possible that the allocation could succeed after acquiring
> > the lock.  If the lock is contended, the task is put to sleep and the
> > allocation attempt is retried when rescheduled.
> 
> The problem with a succeeding allocation is that it takes memory 
> away from the OOM killer?
> 

If the succeeding allocation works with ALLOC_WMARK_HIGH, and 
get_page_from_freelist() returns non-NULL, then clear_zonelist_oom() is 
called and the OOM killer is never invoked.  The same happens if the 
allocation order exceeds PAGE_ALLOC_COSTLY_ORDER.

So, as the comment still says in __alloc_pages(), the succeeding 
allocation attempt is only to catch parallel OOM killings.  Not 
necessarily that we can serialize based on that alone, but it catches 
tasks that were already OOM killed, marked TIF_MEMDIE so they can quickly 
exit, and gone through exit_mm().  This only happens when the earlier 
allocation attempts couldn't catch it because they were ~ALLOC_WMARK_HIGH.

We can't serialize after this final allocation attempt with the new 
try_set_zone_oom() because it is entirely possible that we could enter the 
OOM killer, wait for the read-lock on tasklist_lock, the OOM condition 
could be alleviated, and then we still kill a task unnecessarily.

		David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 3/8] oom: save zonelist pointer for oom killer calls

[Christoph Lameter]

Still think that a simple flag in the zone would be much easier to realize 
and would avoid the kzalloc.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 3/8] oom: save zonelist pointer for oom killer calls

[David Rientjes]

On Wed, 19 Sep 2007, Christoph Lameter wrote:

> Still think that a simple flag in the zone would be much easier to realize 
> and would avoid the kzalloc.
> 

That would require another member to be added to struct zone, probably a 
spinlock_t that we would use as a spin_trylock() when in 
try_set_zone_oom().

Or we could, as you mentioned before, turn all_unreclaimable into an 
unsigned long and use it to set various bits.  That works pretty nicely.

I'm wondering if this OOM killer serialization is going to end up as a 
config option, though.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 3/8] oom: save zonelist pointer for oom killer calls

[Christoph Lameter]

On Wed, 19 Sep 2007, David Rientjes wrote:

> On Wed, 19 Sep 2007, Christoph Lameter wrote:
> 
> > Still think that a simple flag in the zone would be much easier to realize 
> > and would avoid the kzalloc.
> > 
> 
> That would require another member to be added to struct zone, probably a 
> spinlock_t that we would use as a spin_trylock() when in 
> try_set_zone_oom().

> Or we could, as you mentioned before, turn all_unreclaimable into an 
> unsigned long and use it to set various bits.  That works pretty nicely.
> 
> I'm wondering if this OOM killer serialization is going to end up as a 
> config option, though.

Are there any reasons not to serialize the OOM killer per zone?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 3/8] oom: save zonelist pointer for oom killer calls

[David Rientjes]

On Wed, 19 Sep 2007, Christoph Lameter wrote:

> > Or we could, as you mentioned before, turn all_unreclaimable into an 
> > unsigned long and use it to set various bits.  That works pretty nicely.
> > 
> > I'm wondering if this OOM killer serialization is going to end up as a 
> > config option, though.
> 
> Are there any reasons not to serialize the OOM killer per zone?
> 

That's what this current patchset does, yes.  I agree that it is probably 
better done with a bit in struct zone, however.

By changing all_unreclaimable to an unsigned long flags member of struct 
zone, we'll simply need to check_bit() on a new ZONE_OOM flag when we scan 
through the zonelist instead of checking for matching zones from those 
that we saved in try_set_zone_oom().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 3/8] oom: save zonelist pointer for oom killer calls

[Tim Pepper]

On 9/19/07, David Rientjes <rientjes@google.com> wrote:
> On Wed, 19 Sep 2007, Christoph Lameter wrote:
> > Are there any reasons not to serialize the OOM killer per zone?
>
> That's what this current patchset does, yes.  I agree that it is probably
> better done with a bit in struct zone, however.
>

Removing the kzalloc() would be helpful also for the code's
readability in terms of showing (and remembering in the future) its
correctness.  If I've read this right, as it stands try_set_zone_oom()
works out to behaving in the following ways for the listed return
values:

 ret : behaviour
    0: when is_zone_locked() ret's a 1 (ie: because a zone being OOM'd is
       already marked OOM locked),
       NONE of the current zone(s) are added to the list of OOM zones.
    1: when is_zone_locked() ret's all 0's (ie: b/c no zone(s) being OOM'd are
       already marked OOM locked), and the kzalloc() failed,
       NONE of the current zone(s) are added to the list of OOM zones.
    1: when is_zone_locked() ret's all 0's (ie: b/c no zone(s) being OOM'd are
       already marked OOM locked),
       ALL of the current zone(s) are added to the list of OOM zones.

When no zones in the current zonelist are on the list of OOM zones,
then all the current zones are added to the list of OOM zones...or
none of them depending on how badly OOM'd we are.  Tricky.

If any single zone in the current zonelist matches in the list of OOM
zones, none of the current zones are added to the list of OOM zones.
Given the patch header comments, this was done on purpose.  But
doesn't that leave your list of OOM zones incomplete and open you to
OOM killing in parallel on a given zone?

Or is that all ok in that you're trying to minimise needlessly OOM
killing something when possible but are willing to throw in the towel
when things are tending towards royally hosed?

At any rate this seems complex with subtly varying behaviour that left
me wondering if it really works as advertised.  I imagine without the
kzmalloc and instead checking/setting bits in bitmasks the code would
be cleaner.


Tim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 3/8] oom: save zonelist pointer for oom killer calls

[David Rientjes]

On Wed, 19 Sep 2007, Tim Pepper wrote:

> When no zones in the current zonelist are on the list of OOM zones,
> then all the current zones are added to the list of OOM zones...or
> none of them depending on how badly OOM'd we are.  Tricky.
> 

That's not quite how I intended it to be read, but your analysis of the 
return values of try_set_zone_oom() are correct.

It was intended to return non-zero (i.e. the OOM killer is still invoked) 
if the zonelist couldn't be added to our list simply because the kzalloc() 
failed.  This is after the test to see if any of the zones are already 
marked as being in the OOM killer and they weren't (the only reason we 
didn't immediately return 0).  So the return value is correct and the OOM 
killer should still be invoked.

But yeah, it's cleaner if we change all_unreclaimable to an
unsigned int flags and convert all current testers of the 
all_unreclaimable value to use it.  Then we can simply set a bit, 
ZONE_OOM, to identify such zones.

> If any single zone in the current zonelist matches in the list of OOM
> zones, none of the current zones are added to the list of OOM zones.
> Given the patch header comments, this was done on purpose.  But
> doesn't that leave your list of OOM zones incomplete and open you to
> OOM killing in parallel on a given zone?
> 

They aren't added to the list because we aren't going to invoke the OOM 
killer for them, we're going to return 0 to __alloc_pages(), the only 
caller of try_set_zone_oom(), and that will put the task to sleep and then 
retry the allocation that it failed on when it wakes up:

	if (!try_set_zoom_oom(zonelist)) {
		schedule_timeout_uninterruptible(1);
		goto restart;
	}

The only time we return 1 is when none of the zones from the 
__alloc_pages() zonelist was found already to be marked in the OOM killer 
and thus it _prevents_ parallel OOM killings.

> Or is that all ok in that you're trying to minimise needlessly OOM
> killing something when possible but are willing to throw in the towel
> when things are tending towards royally hosed?
> 

The entire patchset is aimed toward serialization and trying to avoid 
needlessly killing tasks when killing one would alleviate the condition.  
The current OOM killer performs very badly for this.

> At any rate this seems complex with subtly varying behaviour that left
> me wondering if it really works as advertised.  I imagine without the
> kzmalloc and instead checking/setting bits in bitmasks the code would
> be cleaner.
> 

There's an easy way to check if it works as advertised, and that's to 
apply the patchset to Linus' latest git and trying it out.  I'm fairly 
happy with the results and I consider it to be much better than the 
current behavior.  I've tested it pretty thoroughly.

But I do agree that checking bits in an unsigned int flags member of 
struct zone will be better, but I intend to still mimic the behavior of a 
trylock for serialization.  try_set_zone_oom() will simply be implemented 
differently.

		David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 3/8] oom: save zonelist pointer for oom killer calls

[Christoph Lameter]

On Wed, 19 Sep 2007, David Rientjes wrote:

> But yeah, it's cleaner if we change all_unreclaimable to an
> unsigned int flags and convert all current testers of the 
> all_unreclaimable value to use it.  Then we can simply set a bit, 
> ZONE_OOM, to identify such zones.

If we do that then we can also get rid of the atomic_t 
reclaim_in_progress. It is only used by zone reclaim these days.

> But I do agree that checking bits in an unsigned int flags member of 
> struct zone will be better, but I intend to still mimic the behavior of a 
> trylock for serialization.  try_set_zone_oom() will simply be implemented 
> differently.

Good.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/8] oom: move prototypes to appropriate header file

[Christoph Lameter]

On Wed, 19 Sep 2007, David Rientjes wrote:

> Move the OOM killer's extern function prototypes to include/linux/oom.h
> and include it where necessary.

Acked-by: Christoph Lameter <clameter@sgi.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 0/8] oom killer updates

[Paul Jackson]

Earlier today, David wrote:
> This patchset is an updated replacement of all patches posted by me to
> linux-mm on September 18, 2007.

To what kernel version does this patch set apply?
I don't see that mentioned in the patch set.

I only tried 2.6.23-rc6-mm1 so far, but got a simple
collision and was too lazy to persue it.

Thanks.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 0/8] oom killer updates

[David Rientjes]

On Wed, 19 Sep 2007, Paul Jackson wrote:

> Earlier today, David wrote:
> > This patchset is an updated replacement of all patches posted by me to
> > linux-mm on September 18, 2007.
> 
> To what kernel version does this patch set apply?
> I don't see that mentioned in the patch set.
> 

It's against Linus' latest git.  I didn't apply it on top of the latest 
-mm because I'm not sure what parts of Andrea's patchset will be applied 
based on consensus.

Sorry for the confusion.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>