linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [patch -mm 1/2] oom: badness heuristic rewrite
@ 2010-07-17 19:16 David Rientjes
  2010-07-17 19:16 ` [patch -mm 2/2] oom: deprecate oom_adj tunable David Rientjes
  2010-07-29 23:08 ` [patch -mm 1/2] oom: badness heuristic rewrite Andrew Morton
  0 siblings, 2 replies; 28+ messages in thread
From: David Rientjes @ 2010-07-17 19:16 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Nick Piggin, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Oleg Nesterov,
	Balbir Singh, linux-mm

This a complete rewrite of the oom killer's badness() heuristic which is
used to determine which task to kill in oom conditions.  The goal is to
make it as simple and predictable as possible so the results are better
understood and we end up killing the task which will lead to the most
memory freeing while still respecting the fine-tuning from userspace.

Instead of basing the heuristic on mm->total_vm for each task, the task's
rss and swap space is used instead.  This is a better indication of the
amount of memory that will be freeable if the oom killed task is chosen
and subsequently exits.  This helps specifically in cases where KDE or
GNOME is chosen for oom kill on desktop systems instead of a memory
hogging task.

The baseline for the heuristic is a proportion of memory that each task is
currently using in memory plus swap compared to the amount of "allowable"
memory.  "Allowable," in this sense, means the system-wide resources for
unconstrained oom conditions, the set of mempolicy nodes, the mems
attached to current's cpuset, or a memory controller's limit.  The
proportion is given on a scale of 0 (never kill) to 1000 (always kill),
roughly meaning that if a task has a badness() score of 500 that the task
consumes approximately 50% of allowable memory resident in RAM or in swap
space.

The proportion is always relative to the amount of "allowable" memory and
not the total amount of RAM systemwide so that mempolicies and cpusets may
operate in isolation; they shall not need to know the true size of the
machine on which they are running if they are bound to a specific set of
nodes or mems, respectively.

Root tasks are given 3% extra memory just like __vm_enough_memory()
provides in LSMs.  In the event of two tasks consuming similar amounts of
memory, it is generally better to save root's task.

Because of the change in the badness() heuristic's baseline, it is also
necessary to introduce a new user interface to tune it.  It's not possible
to redefine the meaning of /proc/pid/oom_adj with a new scale since the
ABI cannot be changed for backward compatability.  Instead, a new tunable,
/proc/pid/oom_score_adj, is added that ranges from -1000 to +1000.  It may
be used to polarize the heuristic such that certain tasks are never
considered for oom kill while others may always be considered.  The value
is added directly into the badness() score so a value of -500, for
example, means to discount 50% of its memory consumption in comparison to
other tasks either on the system, bound to the mempolicy, in the cpuset,
or sharing the same memory controller.

/proc/pid/oom_adj is changed so that its meaning is rescaled into the
units used by /proc/pid/oom_score_adj, and vice versa.  Changing one of
these per-task tunables will rescale the value of the other to an
equivalent meaning.  Although /proc/pid/oom_adj was originally defined as
a bitshift on the badness score, it now shares the same linear growth as
/proc/pid/oom_score_adj but with different granularity.  This is required
so the ABI is not broken with userspace applications and allows oom_adj to
be deprecated for future removal.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 Documentation/filesystems/proc.txt |   94 ++++++++-----
 fs/proc/base.c                     |   94 +++++++++++++-
 include/linux/memcontrol.h         |    8 +
 include/linux/oom.h                |   14 ++-
 include/linux/sched.h              |    3 +-
 kernel/fork.c                      |    1 +
 mm/memcontrol.c                    |   18 +++
 mm/oom_kill.c                      |  258 +++++++++++++++--------------------
 8 files changed, 300 insertions(+), 190 deletions(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -33,7 +33,8 @@ Table of Contents
   2	Modifying System Parameters
 
   3	Per-Process Parameters
-  3.1	/proc/<pid>/oom_adj - Adjust the oom-killer score
+  3.1	/proc/<pid>/oom_adj & /proc/<pid>/oom_score_adj - Adjust the oom-killer
+								score
   3.2	/proc/<pid>/oom_score - Display current oom-killer score
   3.3	/proc/<pid>/io - Display the IO accounting fields
   3.4	/proc/<pid>/coredump_filter - Core dump filtering settings
@@ -1234,42 +1235,61 @@ of the kernel.
 CHAPTER 3: PER-PROCESS PARAMETERS
 ------------------------------------------------------------------------------
 
-3.1 /proc/<pid>/oom_adj - Adjust the oom-killer score
-------------------------------------------------------
-
-This file can be used to adjust the score used to select which processes
-should be killed in an  out-of-memory  situation.  Giving it a high score will
-increase the likelihood of this process being killed by the oom-killer.  Valid
-values are in the range -16 to +15, plus the special value -17, which disables
-oom-killing altogether for this process.
-
-The process to be killed in an out-of-memory situation is selected among all others
-based on its badness score. This value equals the original memory size of the process
-and is then updated according to its CPU time (utime + stime) and the
-run time (uptime - start time). The longer it runs the smaller is the score.
-Badness score is divided by the square root of the CPU time and then by
-the double square root of the run time.
-
-Swapped out tasks are killed first. Half of each child's memory size is added to
-the parent's score if they do not share the same memory. Thus forking servers
-are the prime candidates to be killed. Having only one 'hungry' child will make
-parent less preferable than the child.
-
-/proc/<pid>/oom_score shows process' current badness score.
-
-The following heuristics are then applied:
- * if the task was reniced, its score doubles
- * superuser or direct hardware access tasks (CAP_SYS_ADMIN, CAP_SYS_RESOURCE
- 	or CAP_SYS_RAWIO) have their score divided by 4
- * if oom condition happened in one cpuset and checked process does not belong
- 	to it, its score is divided by 8
- * the resulting score is multiplied by two to the power of oom_adj, i.e.
-	points <<= oom_adj when it is positive and
-	points >>= -(oom_adj) otherwise
-
-The task with the highest badness score is then selected and its children
-are killed, process itself will be killed in an OOM situation when it does
-not have children or some of them disabled oom like described above.
+3.1 /proc/<pid>/oom_adj & /proc/<pid>/oom_score_adj- Adjust the oom-killer score
+--------------------------------------------------------------------------------
+
+These file can be used to adjust the badness heuristic used to select which
+process gets killed in out of memory conditions.
+
+The badness heuristic assigns a value to each candidate task ranging from 0
+(never kill) to 1000 (always kill) to determine which process is targeted.  The
+units are roughly a proportion along that range of allowed memory the process
+may allocate from based on an estimation of its current memory and swap use.
+For example, if a task is using all allowed memory, its badness score will be
+1000.  If it is using half of its allowed memory, its score will be 500.
+
+There is an additional factor included in the badness score: root
+processes are given 3% extra memory over other tasks.
+
+The amount of "allowed" memory depends on the context in which the oom killer
+was called.  If it is due to the memory assigned to the allocating task's cpuset
+being exhausted, the allowed memory represents the set of mems assigned to that
+cpuset.  If it is due to a mempolicy's node(s) being exhausted, the allowed
+memory represents the set of mempolicy nodes.  If it is due to a memory
+limit (or swap limit) being reached, the allowed memory is that configured
+limit.  Finally, if it is due to the entire system being out of memory, the
+allowed memory represents all allocatable resources.
+
+The value of /proc/<pid>/oom_score_adj is added to the badness score before it
+is used to determine which task to kill.  Acceptable values range from -1000
+(OOM_SCORE_ADJ_MIN) to +1000 (OOM_SCORE_ADJ_MAX).  This allows userspace to
+polarize the preference for oom killing either by always preferring a certain
+task or completely disabling it.  The lowest possible value, -1000, is
+equivalent to disabling oom killing entirely for that task since it will always
+report a badness score of 0.
+
+Consequently, it is very simple for userspace to define the amount of memory to
+consider for each task.  Setting a /proc/<pid>/oom_score_adj value of +500, for
+example, is roughly equivalent to allowing the remainder of tasks sharing the
+same system, cpuset, mempolicy, or memory controller resources to use at least
+50% more memory.  A value of -500, on the other hand, would be roughly
+equivalent to discounting 50% of the task's allowed memory from being considered
+as scoring against the task.
+
+For backwards compatibility with previous kernels, /proc/<pid>/oom_adj may also
+be used to tune the badness score.  Its acceptable values range from -16
+(OOM_ADJUST_MIN) to +15 (OOM_ADJUST_MAX) and a special value of -17
+(OOM_DISABLE) to disable oom killing entirely for that task.  Its value is
+scaled linearly with /proc/<pid>/oom_score_adj.
+
+Writing to /proc/<pid>/oom_score_adj or /proc/<pid>/oom_adj will change the
+other with its scaled value.
+
+Caveat: when a parent task is selected, the oom killer will sacrifice any first
+generation children with seperate address spaces instead, if possible.  This
+avoids servers and important system daemons from being killed and loses the
+minimal amount of work.
+
 
 3.2 /proc/<pid>/oom_score - Display current oom-killer score
 -------------------------------------------------------------
diff --git a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -63,6 +63,7 @@
 #include <linux/namei.h>
 #include <linux/mnt_namespace.h>
 #include <linux/mm.h>
+#include <linux/swap.h>
 #include <linux/rcupdate.h>
 #include <linux/kallsyms.h>
 #include <linux/stacktrace.h>
@@ -430,12 +431,11 @@ static const struct file_operations proc_lstats_operations = {
 static int proc_oom_score(struct task_struct *task, char *buffer)
 {
 	unsigned long points = 0;
-	struct timespec uptime;
 
-	do_posix_clock_monotonic_gettime(&uptime);
 	read_lock(&tasklist_lock);
 	if (pid_alive(task))
-		points = badness(task, NULL, NULL, uptime.tv_sec);
+		points = oom_badness(task, NULL, NULL,
+					totalram_pages + total_swap_pages);
 	read_unlock(&tasklist_lock);
 	return sprintf(buffer, "%lu\n", points);
 }
@@ -1050,7 +1050,15 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf,
 	}
 
 	task->signal->oom_adj = oom_adjust;
-
+	/*
+	 * Scale /proc/pid/oom_score_adj appropriately ensuring that a maximum
+	 * value is always attainable.
+	 */
+	if (task->signal->oom_adj == OOM_ADJUST_MAX)
+		task->signal->oom_score_adj = OOM_SCORE_ADJ_MAX;
+	else
+		task->signal->oom_score_adj = (oom_adjust * OOM_SCORE_ADJ_MAX) /
+								-OOM_DISABLE;
 	unlock_task_sighand(task, &flags);
 	put_task_struct(task);
 
@@ -1063,6 +1071,82 @@ static const struct file_operations proc_oom_adjust_operations = {
 	.llseek		= generic_file_llseek,
 };
 
+static ssize_t oom_score_adj_read(struct file *file, char __user *buf,
+					size_t count, loff_t *ppos)
+{
+	struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode);
+	char buffer[PROC_NUMBUF];
+	int oom_score_adj = OOM_SCORE_ADJ_MIN;
+	unsigned long flags;
+	size_t len;
+
+	if (!task)
+		return -ESRCH;
+	if (lock_task_sighand(task, &flags)) {
+		oom_score_adj = task->signal->oom_score_adj;
+		unlock_task_sighand(task, &flags);
+	}
+	put_task_struct(task);
+	len = snprintf(buffer, sizeof(buffer), "%d\n", oom_score_adj);
+	return simple_read_from_buffer(buf, count, ppos, buffer, len);
+}
+
+static ssize_t oom_score_adj_write(struct file *file, const char __user *buf,
+					size_t count, loff_t *ppos)
+{
+	struct task_struct *task;
+	char buffer[PROC_NUMBUF];
+	unsigned long flags;
+	long oom_score_adj;
+	int err;
+
+	memset(buffer, 0, sizeof(buffer));
+	if (count > sizeof(buffer) - 1)
+		count = sizeof(buffer) - 1;
+	if (copy_from_user(buffer, buf, count))
+		return -EFAULT;
+
+	err = strict_strtol(strstrip(buffer), 0, &oom_score_adj);
+	if (err)
+		return -EINVAL;
+	if (oom_score_adj < OOM_SCORE_ADJ_MIN ||
+			oom_score_adj > OOM_SCORE_ADJ_MAX)
+		return -EINVAL;
+
+	task = get_proc_task(file->f_path.dentry->d_inode);
+	if (!task)
+		return -ESRCH;
+	if (!lock_task_sighand(task, &flags)) {
+		put_task_struct(task);
+		return -ESRCH;
+	}
+	if (oom_score_adj < task->signal->oom_score_adj &&
+			!capable(CAP_SYS_RESOURCE)) {
+		unlock_task_sighand(task, &flags);
+		put_task_struct(task);
+		return -EACCES;
+	}
+
+	task->signal->oom_score_adj = oom_score_adj;
+	/*
+	 * Scale /proc/pid/oom_adj appropriately ensuring that OOM_DISABLE is
+	 * always attainable.
+	 */
+	if (task->signal->oom_score_adj == OOM_SCORE_ADJ_MIN)
+		task->signal->oom_adj = OOM_DISABLE;
+	else
+		task->signal->oom_adj = (oom_score_adj * OOM_ADJUST_MAX) /
+							OOM_SCORE_ADJ_MAX;
+	unlock_task_sighand(task, &flags);
+	put_task_struct(task);
+	return count;
+}
+
+static const struct file_operations proc_oom_score_adj_operations = {
+	.read		= oom_score_adj_read,
+	.write		= oom_score_adj_write,
+};
+
 #ifdef CONFIG_AUDITSYSCALL
 #define TMPBUFLEN 21
 static ssize_t proc_loginuid_read(struct file * file, char __user * buf,
@@ -2635,6 +2719,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 #endif
 	INF("oom_score",  S_IRUGO, proc_oom_score),
 	REG("oom_adj",    S_IRUGO|S_IWUSR, proc_oom_adjust_operations),
+	REG("oom_score_adj", S_IRUGO|S_IWUSR, proc_oom_score_adj_operations),
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
@@ -2969,6 +3054,7 @@ static const struct pid_entry tid_base_stuff[] = {
 #endif
 	INF("oom_score", S_IRUGO, proc_oom_score),
 	REG("oom_adj",   S_IRUGO|S_IWUSR, proc_oom_adjust_operations),
+	REG("oom_score_adj", S_IRUGO|S_IWUSR, proc_oom_score_adj_operations),
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",  S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUSR, proc_sessionid_operations),
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -130,6 +130,8 @@ void mem_cgroup_update_file_mapped(struct page *page, int val);
 unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,
 						gfp_t gfp_mask, int nid,
 						int zid);
+u64 mem_cgroup_get_limit(struct mem_cgroup *mem);
+
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct mem_cgroup;
 
@@ -309,6 +311,12 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,
 	return 0;
 }
 
+static inline
+u64 mem_cgroup_get_limit(struct mem_cgroup *mem)
+{
+	return 0;
+}
+
 #endif /* CONFIG_CGROUP_MEM_CONT */
 
 #endif /* _LINUX_MEMCONTROL_H */
diff --git a/include/linux/oom.h b/include/linux/oom.h
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -1,14 +1,24 @@
 #ifndef __INCLUDE_LINUX_OOM_H
 #define __INCLUDE_LINUX_OOM_H
 
-/* /proc/<pid>/oom_adj set to -17 protects from the oom-killer */
+/*
+ * /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
 
+/*
+ * /proc/<pid>/oom_score_adj set to OOM_SCORE_ADJ_MIN disables oom killing for
+ * pid.
+ */
+#define OOM_SCORE_ADJ_MIN	(-1000)
+#define OOM_SCORE_ADJ_MAX	1000
+
 #ifdef __KERNEL__
 
+#include <linux/sched.h>
 #include <linux/types.h>
 #include <linux/nodemask.h>
 
@@ -27,6 +37,8 @@ enum oom_constraint {
 	CONSTRAINT_MEMCG,
 };
 
+extern unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *mem,
+			const nodemask_t *nodemask, unsigned long totalpages);
 extern int try_set_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
 extern void clear_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -626,7 +626,8 @@ struct signal_struct {
 	struct tty_audit_buf *tty_audit_buf;
 #endif
 
-	int oom_adj;	/* OOM kill score adjustment (bit shift) */
+	int oom_adj;		/* OOM kill score adjustment (bit shift) */
+	int oom_score_adj;	/* OOM kill score adjustment */
 };
 
 /* Context switch must be unlocked if interrupts are to be enabled */
diff --git a/kernel/fork.c b/kernel/fork.c
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -899,6 +899,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
 	tty_audit_fork(sig);
 
 	sig->oom_adj = current->signal->oom_adj;
+	sig->oom_score_adj = current->signal->oom_score_adj;
 
 	return 0;
 }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1203,6 +1203,24 @@ static int mem_cgroup_count_children(struct mem_cgroup *mem)
 }
 
 /*
+ * Return the memory (and swap, if configured) limit for a memcg.
+ */
+u64 mem_cgroup_get_limit(struct mem_cgroup *memcg)
+{
+	u64 limit;
+	u64 memsw;
+
+	limit = res_counter_read_u64(&memcg->res, RES_LIMIT) +
+			total_swap_pages;
+	memsw = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
+	/*
+	 * If memsw is finite and limits the amount of swap space available
+	 * to this memcg, return that limit.
+	 */
+	return min(limit, memsw);
+}
+
+/*
  * Visit the first child (need not be the first child as per the ordering
  * of the cgroup list, since we track last_scanned_child) of @mem and use
  * that to reclaim free pages from.
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -4,6 +4,8 @@
  *  Copyright (C)  1998,2000  Rik van Riel
  *	Thanks go out to Claus Fischer for some serious inspiration and
  *	for goading me into coding this file...
+ *  Copyright (C)  2010  Google, Inc.
+ *	Rewritten by David Rientjes
  *
  *  The routines in this file are used to kill a process when
  *  we're seriously out of memory. This gets called from __alloc_pages()
@@ -34,7 +36,6 @@ int sysctl_panic_on_oom;
 int sysctl_oom_kill_allocating_task;
 int sysctl_oom_dump_tasks = 1;
 static DEFINE_SPINLOCK(zone_scan_lock);
-/* #define DEBUG */
 
 #ifdef CONFIG_NUMA
 /**
@@ -140,137 +141,76 @@ static bool oom_unkillable_task(struct task_struct *p, struct mem_cgroup *mem,
 }
 
 /**
- * badness - calculate a numeric value for how bad this task has been
+ * oom_badness - heuristic function to determine which candidate task to kill
  * @p: task struct of which task we should calculate
- * @uptime: current uptime in seconds
+ * @totalpages: total present RAM allowed for page allocation
  *
- * The formula used is relatively simple and documented inline in the
- * function. The main rationale is that we want to select a good task
- * to kill when we run out of memory.
- *
- * Good in this context means that:
- * 1) we lose the minimum amount of work done
- * 2) we recover a large amount of memory
- * 3) we don't kill anything innocent of eating tons of memory
- * 4) we want to kill the minimum amount of processes (one)
- * 5) we try to kill the process the user expects us to kill, this
- *    algorithm has been meticulously tuned to meet the principle
- *    of least surprise ... (be careful when you change it)
+ * The heuristic for determining which task to kill is made to be as simple and
+ * predictable as possible.  The goal is to return the highest value for the
+ * task consuming the most memory to avoid subsequent oom failures.
  */
-unsigned long badness(struct task_struct *p, struct mem_cgroup *mem,
-		      const nodemask_t *nodemask, unsigned long uptime)
+unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *mem,
+		      const nodemask_t *nodemask, unsigned long totalpages)
 {
-	unsigned long points, cpu_time, run_time;
-	struct task_struct *child;
-	struct task_struct *c, *t;
-	int oom_adj = p->signal->oom_adj;
-	struct task_cputime task_time;
-	unsigned long utime;
-	unsigned long stime;
+	int points;
 
 	if (oom_unkillable_task(p, mem, nodemask))
 		return 0;
-	if (oom_adj == OOM_DISABLE)
-		return 0;
 
 	p = find_lock_task_mm(p);
 	if (!p)
 		return 0;
 
 	/*
-	 * The memory size of the process is the basis for the badness.
-	 */
-	points = p->mm->total_vm;
-	task_unlock(p);
-
-	/*
-	 * swapoff can easily use up all memory, so kill those first.
-	 */
-	if (p->flags & PF_OOM_ORIGIN)
-		return ULONG_MAX;
-
-	/*
-	 * Processes which fork a lot of child processes are likely
-	 * a good choice. We add half the vmsize of the children if they
-	 * have an own mm. This prevents forking servers to flood the
-	 * machine with an endless amount of children. In case a single
-	 * child is eating the vast majority of memory, adding only half
-	 * to the parents will make the child our kill candidate of choice.
+	 * Shortcut check for OOM_SCORE_ADJ_MIN so the entire heuristic doesn't
+	 * need to be executed for something that cannot be killed.
 	 */
-	t = p;
-	do {
-		list_for_each_entry(c, &t->children, sibling) {
-			child = find_lock_task_mm(c);
-			if (child) {
-				if (child->mm != p->mm)
-					points += child->mm->total_vm/2 + 1;
-				task_unlock(child);
-			}
-		}
-	} while_each_thread(p, t);
+	if (p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
+		task_unlock(p);
+		return 0;
+	}
 
 	/*
-	 * CPU time is in tens of seconds and run time is in thousands
-         * of seconds. There is no particular reason for this other than
-         * that it turned out to work very well in practice.
+	 * When the PF_OOM_ORIGIN bit is set, it indicates the task should have
+	 * priority for oom killing.
 	 */
-	thread_group_cputime(p, &task_time);
-	utime = cputime_to_jiffies(task_time.utime);
-	stime = cputime_to_jiffies(task_time.stime);
-	cpu_time = (utime + stime) >> (SHIFT_HZ + 3);
-
-
-	if (uptime >= p->start_time.tv_sec)
-		run_time = (uptime - p->start_time.tv_sec) >> 10;
-	else
-		run_time = 0;
-
-	if (cpu_time)
-		points /= int_sqrt(cpu_time);
-	if (run_time)
-		points /= int_sqrt(int_sqrt(run_time));
+	if (p->flags & PF_OOM_ORIGIN) {
+		task_unlock(p);
+		return 1000;
+	}
 
 	/*
-	 * Niced processes are most likely less important, so double
-	 * their badness points.
+	 * The memory controller may have a limit of 0 bytes, so avoid a divide
+	 * by zero, if necessary.
 	 */
-	if (task_nice(p) > 0)
-		points *= 2;
+	if (!totalpages)
+		totalpages = 1;
 
 	/*
-	 * Superuser processes are usually more important, so we make it
-	 * less likely that we kill those.
+	 * The baseline for the badness score is the proportion of RAM that each
+	 * task's rss and swap space use.
 	 */
-	if (has_capability_noaudit(p, CAP_SYS_ADMIN) ||
-	    has_capability_noaudit(p, CAP_SYS_RESOURCE))
-		points /= 4;
+	points = (get_mm_rss(p->mm) + get_mm_counter(p->mm, MM_SWAPENTS)) * 1000 /
+			totalpages;
+	task_unlock(p);
 
 	/*
-	 * We don't want to kill a process with direct hardware access.
-	 * Not only could that mess up the hardware, but usually users
-	 * tend to only have this flag set on applications they think
-	 * of as important.
+	 * Root processes get 3% bonus, just like the __vm_enough_memory()
+	 * implementation used by LSMs.
 	 */
-	if (has_capability_noaudit(p, CAP_SYS_RAWIO))
-		points /= 4;
+	if (has_capability_noaudit(p, CAP_SYS_ADMIN))
+		points -= 30;
 
 	/*
-	 * Adjust the score by oom_adj.
+	 * /proc/pid/oom_score_adj ranges from -1000 to +1000 such that it may
+	 * either completely disable oom killing or always prefer a certain
+	 * task.
 	 */
-	if (oom_adj) {
-		if (oom_adj > 0) {
-			if (!points)
-				points = 1;
-			points <<= oom_adj;
-		} else
-			points >>= -(oom_adj);
-	}
+	points += p->signal->oom_score_adj;
 
-#ifdef DEBUG
-	printk(KERN_DEBUG "OOMkill: task %d (%s) got %lu points\n",
-	p->pid, p->comm, points);
-#endif
-	return points;
+	if (points < 0)
+		return 0;
+	return (points < 1000) ? points : 1000;
 }
 
 /*
@@ -278,12 +218,20 @@ unsigned long badness(struct task_struct *p, struct mem_cgroup *mem,
  */
 #ifdef CONFIG_NUMA
 static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
-				    gfp_t gfp_mask, nodemask_t *nodemask)
+				gfp_t gfp_mask, nodemask_t *nodemask,
+				unsigned long *totalpages)
 {
 	struct zone *zone;
 	struct zoneref *z;
 	enum zone_type high_zoneidx = gfp_zone(gfp_mask);
+	bool cpuset_limited = false;
+	int nid;
 
+	/* Default to all available memory */
+	*totalpages = totalram_pages + total_swap_pages;
+
+	if (!zonelist)
+		return CONSTRAINT_NONE;
 	/*
 	 * Reach here only when __GFP_NOFAIL is used. So, we should avoid
 	 * to kill current.We have to random task kill in this case.
@@ -293,26 +241,37 @@ static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
 		return CONSTRAINT_NONE;
 
 	/*
-	 * The nodemask here is a nodemask passed to alloc_pages(). Now,
-	 * cpuset doesn't use this nodemask for its hardwall/softwall/hierarchy
-	 * feature. mempolicy is an only user of nodemask here.
-	 * check mempolicy's nodemask contains all N_HIGH_MEMORY
+	 * This is not a __GFP_THISNODE allocation, so a truncated nodemask in
+	 * the page allocator means a mempolicy is in effect.  Cpuset policy
+	 * is enforced in get_page_from_freelist().
 	 */
-	if (nodemask && !nodes_subset(node_states[N_HIGH_MEMORY], *nodemask))
+	if (nodemask && !nodes_subset(node_states[N_HIGH_MEMORY], *nodemask)) {
+		*totalpages = total_swap_pages;
+		for_each_node_mask(nid, *nodemask)
+			*totalpages += node_spanned_pages(nid);
 		return CONSTRAINT_MEMORY_POLICY;
+	}
 
 	/* Check this allocation failure is caused by cpuset's wall function */
 	for_each_zone_zonelist_nodemask(zone, z, zonelist,
 			high_zoneidx, nodemask)
 		if (!cpuset_zone_allowed_softwall(zone, gfp_mask))
-			return CONSTRAINT_CPUSET;
+			cpuset_limited = true;
 
+	if (cpuset_limited) {
+		*totalpages = total_swap_pages;
+		for_each_node_mask(nid, cpuset_current_mems_allowed)
+			*totalpages += node_spanned_pages(nid);
+		return CONSTRAINT_CPUSET;
+	}
 	return CONSTRAINT_NONE;
 }
 #else
 static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
-				gfp_t gfp_mask, nodemask_t *nodemask)
+				gfp_t gfp_mask, nodemask_t *nodemask,
+				unsigned long *totalpages)
 {
+	*totalpages = totalram_pages + total_swap_pages;
 	return CONSTRAINT_NONE;
 }
 #endif
@@ -323,17 +282,16 @@ static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
  *
  * (not docbooked, we don't want this one cluttering up the manual)
  */
-static struct task_struct *select_bad_process(unsigned long *ppoints,
-		struct mem_cgroup *mem, const nodemask_t *nodemask)
+static struct task_struct *select_bad_process(unsigned int *ppoints,
+		unsigned long totalpages, struct mem_cgroup *mem,
+		const nodemask_t *nodemask)
 {
 	struct task_struct *p;
 	struct task_struct *chosen = NULL;
-	struct timespec uptime;
 	*ppoints = 0;
 
-	do_posix_clock_monotonic_gettime(&uptime);
 	for_each_process(p) {
-		unsigned long points;
+		unsigned int points;
 
 		if (oom_unkillable_task(p, mem, nodemask))
 			continue;
@@ -365,11 +323,11 @@ static struct task_struct *select_bad_process(unsigned long *ppoints,
 				return ERR_PTR(-1UL);
 
 			chosen = p;
-			*ppoints = ULONG_MAX;
+			*ppoints = 1000;
 		}
 
-		points = badness(p, mem, nodemask, uptime.tv_sec);
-		if (points > *ppoints || !chosen) {
+		points = oom_badness(p, mem, nodemask, totalpages);
+		if (points > *ppoints) {
 			chosen = p;
 			*ppoints = points;
 		}
@@ -384,7 +342,7 @@ static struct task_struct *select_bad_process(unsigned long *ppoints,
  *
  * Dumps the current memory state of all system tasks, excluding kernel threads.
  * State information includes task's pid, uid, tgid, vm size, rss, cpu, oom_adj
- * score, and name.
+ * value, oom_score_adj value, and name.
  *
  * If the actual is non-NULL, only tasks that are a member of the mem_cgroup are
  * shown.
@@ -396,8 +354,7 @@ static void dump_tasks(const struct mem_cgroup *mem)
 	struct task_struct *p;
 	struct task_struct *task;
 
-	printk(KERN_INFO "[ pid ]   uid  tgid total_vm      rss cpu oom_adj "
-	       "name\n");
+	pr_info("[ pid ]   uid  tgid total_vm      rss cpu oom_adj oom_score_adj name\n");
 	for_each_process(p) {
 		if (p->flags & PF_KTHREAD)
 			continue;
@@ -414,10 +371,11 @@ static void dump_tasks(const struct mem_cgroup *mem)
 			continue;
 		}
 
-		printk(KERN_INFO "[%5d] %5d %5d %8lu %8lu %3u     %3d %s\n",
-		       task->pid, __task_cred(task)->uid, task->tgid,
-		       task->mm->total_vm, get_mm_rss(task->mm),
-		       task_cpu(task), task->signal->oom_adj, task->comm);
+		pr_info("[%5d] %5d %5d %8lu %8lu %3u     %3d         %5d %s\n",
+			task->pid, __task_cred(task)->uid, task->tgid,
+			task->mm->total_vm, get_mm_rss(task->mm),
+			task_cpu(task), task->signal->oom_adj,
+			task->signal->oom_score_adj, task->comm);
 		task_unlock(task);
 	}
 }
@@ -427,8 +385,9 @@ static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order,
 {
 	task_lock(current);
 	pr_warning("%s invoked oom-killer: gfp_mask=0x%x, order=%d, "
-		"oom_adj=%d\n",
-		current->comm, gfp_mask, order, current->signal->oom_adj);
+		"oom_adj=%d, oom_score_adj=%d\n",
+		current->comm, gfp_mask, order, current->signal->oom_adj,
+		current->signal->oom_score_adj);
 	cpuset_print_task_mems_allowed(current);
 	task_unlock(current);
 	dump_stack();
@@ -468,14 +427,14 @@ static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
 #undef K
 
 static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
-			    unsigned long points, struct mem_cgroup *mem,
-			    nodemask_t *nodemask, const char *message)
+			    unsigned int points, unsigned long totalpages,
+			    struct mem_cgroup *mem, nodemask_t *nodemask,
+			    const char *message)
 {
 	struct task_struct *victim = p;
 	struct task_struct *child;
 	struct task_struct *t = p;
-	unsigned long victim_points = 0;
-	struct timespec uptime;
+	unsigned int victim_points = 0;
 
 	if (printk_ratelimit())
 		dump_header(p, gfp_mask, order, mem);
@@ -491,7 +450,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 	}
 
 	task_lock(p);
-	pr_err("%s: Kill process %d (%s) score %lu or sacrifice child\n",
+	pr_err("%s: Kill process %d (%s) score %d or sacrifice child\n",
 		message, task_pid_nr(p), p->comm, points);
 	task_unlock(p);
 
@@ -501,14 +460,15 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 	 * parent.  This attempts to lose the minimal amount of work done while
 	 * still freeing memory.
 	 */
-	do_posix_clock_monotonic_gettime(&uptime);
 	do {
 		list_for_each_entry(child, &t->children, sibling) {
-			unsigned long child_points;
+			unsigned int child_points;
 
-			/* badness() returns 0 if the thread is unkillable */
-			child_points = badness(child, mem, nodemask,
-					       uptime.tv_sec);
+			/*
+			 * oom_badness() returns 0 if the thread is unkillable
+			 */
+			child_points = oom_badness(child, mem, nodemask,
+								totalpages);
 			if (child_points > victim_points) {
 				victim = child;
 				victim_points = child_points;
@@ -546,17 +506,19 @@ static void check_panic_on_oom(enum oom_constraint constraint, gfp_t gfp_mask,
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR
 void mem_cgroup_out_of_memory(struct mem_cgroup *mem, gfp_t gfp_mask)
 {
-	unsigned long points = 0;
+	unsigned long limit;
+	unsigned int points = 0;
 	struct task_struct *p;
 
 	check_panic_on_oom(CONSTRAINT_MEMCG, gfp_mask, 0);
+	limit = mem_cgroup_get_limit(mem) >> PAGE_SHIFT;
 	read_lock(&tasklist_lock);
 retry:
-	p = select_bad_process(&points, mem, NULL);
+	p = select_bad_process(&points, limit, mem, NULL);
 	if (!p || PTR_ERR(p) == -1UL)
 		goto out;
 
-	if (oom_kill_process(p, gfp_mask, 0, points, mem, NULL,
+	if (oom_kill_process(p, gfp_mask, 0, points, limit, mem, NULL,
 				"Memory cgroup out of memory"))
 		goto retry;
 out:
@@ -681,8 +643,9 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
 		int order, nodemask_t *nodemask)
 {
 	struct task_struct *p;
+	unsigned long totalpages;
 	unsigned long freed = 0;
-	unsigned long points;
+	unsigned int points;
 	enum oom_constraint constraint = CONSTRAINT_NONE;
 
 	blocking_notifier_call_chain(&oom_notify_list, 0, &freed);
@@ -705,8 +668,8 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
 	 * Check if there were limitations on the allocation (only relevant for
 	 * NUMA) that may require different handling.
 	 */
-	if (zonelist)
-		constraint = constrained_alloc(zonelist, gfp_mask, nodemask);
+	constraint = constrained_alloc(zonelist, gfp_mask, nodemask,
+						&totalpages);
 	check_panic_on_oom(constraint, gfp_mask, order);
 
 	read_lock(&tasklist_lock);
@@ -718,13 +681,14 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
 		 * non-zero, current could not be killed so we must fallback to
 		 * the tasklist scan.
 		 */
-		if (!oom_kill_process(current, gfp_mask, order, 0, NULL, nodemask,
+		if (!oom_kill_process(current, gfp_mask, order, 0, totalpages,
+				NULL, nodemask,
 				"Out of memory (oom_kill_allocating_task)"))
 			return;
 	}
 
 retry:
-	p = select_bad_process(&points, NULL,
+	p = select_bad_process(&points, totalpages, NULL,
 			constraint == CONSTRAINT_MEMORY_POLICY ? nodemask :
 								 NULL);
 	if (PTR_ERR(p) == -1UL)
@@ -737,8 +701,8 @@ retry:
 		panic("Out of memory and no killable processes...\n");
 	}
 
-	if (oom_kill_process(p, gfp_mask, order, points, NULL, nodemask,
-			     "Out of memory"))
+	if (oom_kill_process(p, gfp_mask, order, points, totalpages, NULL,
+				nodemask, "Out of memory"))
 		goto retry;
 	read_unlock(&tasklist_lock);
 

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [patch -mm 2/2] oom: deprecate oom_adj tunable
  2010-07-17 19:16 [patch -mm 1/2] oom: badness heuristic rewrite David Rientjes
@ 2010-07-17 19:16 ` David Rientjes
  2010-07-29 23:08 ` [patch -mm 1/2] oom: badness heuristic rewrite Andrew Morton
  1 sibling, 0 replies; 28+ messages in thread
From: David Rientjes @ 2010-07-17 19:16 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Nick Piggin, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Oleg Nesterov,
	Balbir Singh, linux-mm

/proc/pid/oom_adj is now deprecated so that that it may eventually be
removed.  The target date for removal is August 2012.

A warning will be printed to the kernel log if a task attempts to use this
interface.  Future warning will be suppressed until the kernel is rebooted
to prevent spamming the kernel log.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 Documentation/feature-removal-schedule.txt |   25 +++++++++++++++++++++++++
 Documentation/filesystems/proc.txt         |    3 +++
 fs/proc/base.c                             |    8 ++++++++
 include/linux/oom.h                        |    3 +++
 4 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -174,6 +174,31 @@ Who:	Eric Biederman <ebiederm@xmission.com>
 
 ---------------------------
 
+What:	/proc/<pid>/oom_adj
+When:	August 2012
+Why:	/proc/<pid>/oom_adj allows userspace to influence the oom killer's
+	badness heuristic used to determine which task to kill when the kernel
+	is out of memory.
+
+	The badness heuristic has since been rewritten since the introduction of
+	this tunable such that its meaning is deprecated.  The value was
+	implemented as a bitshift on a score generated by the badness()
+	function that did not have any precise units of measure.  With the
+	rewrite, the score is given as a proportion of available memory to the
+	task allocating pages, so using a bitshift which grows the score
+	exponentially is, thus, impossible to tune with fine granularity.
+
+	A much more powerful interface, /proc/<pid>/oom_score_adj, was
+	introduced with the oom killer rewrite that allows users to increase or
+	decrease the badness() score linearly.  This interface will replace
+	/proc/<pid>/oom_adj.
+
+	A warning will be emitted to the kernel log if an application uses this
+	deprecated interface.  After it is printed once, future warnings will be
+	suppressed until the kernel is rebooted.
+
+---------------------------
+
 What:	remove EXPORT_SYMBOL(kernel_thread)
 When:	August 2006
 Files:	arch/*/kernel/*_ksyms.c
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -1285,6 +1285,9 @@ scaled linearly with /proc/<pid>/oom_score_adj.
 Writing to /proc/<pid>/oom_score_adj or /proc/<pid>/oom_adj will change the
 other with its scaled value.
 
+NOTICE: /proc/<pid>/oom_adj is deprecated and will be removed, please see
+Documentation/feature-removal-schedule.txt.
+
 Caveat: when a parent task is selected, the oom killer will sacrifice any first
 generation children with seperate address spaces instead, if possible.  This
 avoids servers and important system daemons from being killed and loses the
diff --git a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1049,6 +1049,14 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf,
 		return -EACCES;
 	}
 
+	/*
+	 * Warn that /proc/pid/oom_adj is deprecated, see
+	 * Documentation/feature-removal-schedule.txt.
+	 */
+	printk_once(KERN_WARNING "%s (%d): /proc/%d/oom_adj is deprecated, "
+			"please use /proc/%d/oom_score_adj instead.\n",
+			current->comm, task_pid_nr(current),
+			task_pid_nr(task), task_pid_nr(task));
 	task->signal->oom_adj = oom_adjust;
 	/*
 	 * Scale /proc/pid/oom_score_adj appropriately ensuring that a maximum
diff --git a/include/linux/oom.h b/include/linux/oom.h
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -2,6 +2,9 @@
 #define __INCLUDE_LINUX_OOM_H
 
 /*
+ * /proc/<pid>/oom_adj is deprecated, see
+ * Documentation/feature-removal-schedule.txt.
+ *
  * /proc/<pid>/oom_adj set to -17 protects from the oom-killer
  */
 #define OOM_DISABLE (-17)

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-07-17 19:16 [patch -mm 1/2] oom: badness heuristic rewrite David Rientjes
  2010-07-17 19:16 ` [patch -mm 2/2] oom: deprecate oom_adj tunable David Rientjes
@ 2010-07-29 23:08 ` Andrew Morton
  2010-07-30  0:12   ` KOSAKI Motohiro
  1 sibling, 1 reply; 28+ messages in thread
From: Andrew Morton @ 2010-07-29 23:08 UTC (permalink / raw)
  To: David Rientjes
  Cc: Nick Piggin, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Oleg Nesterov,
	Balbir Singh, linux-mm

On Sat, 17 Jul 2010 12:16:33 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:

> This a complete rewrite of the oom killer's badness() heuristic 

Any comments here, or are we ready to proceed?

Gimme those acked-bys, reviewed-bys and tested-bys, please!

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-07-29 23:08 ` [patch -mm 1/2] oom: badness heuristic rewrite Andrew Morton
@ 2010-07-30  0:12   ` KOSAKI Motohiro
  2010-07-30  1:38     ` Andrew Morton
  0 siblings, 1 reply; 28+ messages in thread
From: KOSAKI Motohiro @ 2010-07-30  0:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kosaki.motohiro, David Rientjes, Nick Piggin, KAMEZAWA Hiroyuki,
	Oleg Nesterov, Balbir Singh, linux-mm

> On Sat, 17 Jul 2010 12:16:33 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
> 
> > This a complete rewrite of the oom killer's badness() heuristic 
> 
> Any comments here, or are we ready to proceed?
> 
> Gimme those acked-bys, reviewed-bys and tested-bys, please!

If he continue to resend all of rewrite patch, I continue to refuse them.
I explained it multi times.




--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-07-30  0:12   ` KOSAKI Motohiro
@ 2010-07-30  1:38     ` Andrew Morton
  2010-07-30 11:02       ` KOSAKI Motohiro
  0 siblings, 1 reply; 28+ messages in thread
From: Andrew Morton @ 2010-07-30  1:38 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: David Rientjes, Nick Piggin, KAMEZAWA Hiroyuki, Oleg Nesterov,
	Balbir Singh, linux-mm

On Fri, 30 Jul 2010 09:12:26 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> > On Sat, 17 Jul 2010 12:16:33 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
> > 
> > > This a complete rewrite of the oom killer's badness() heuristic 
> > 
> > Any comments here, or are we ready to proceed?
> > 
> > Gimme those acked-bys, reviewed-bys and tested-bys, please!
> 
> If he continue to resend all of rewrite patch, I continue to refuse them.
> I explained it multi times.

There are about 1000 emails on this topic.  Please briefly explain it again.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-07-30  1:38     ` Andrew Morton
@ 2010-07-30 11:02       ` KOSAKI Motohiro
  2010-07-30 20:14         ` David Rientjes
  2010-08-02 20:43         ` Andrew Morton
  0 siblings, 2 replies; 28+ messages in thread
From: KOSAKI Motohiro @ 2010-07-30 11:02 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kosaki.motohiro, David Rientjes, Nick Piggin, KAMEZAWA Hiroyuki,
	Oleg Nesterov, Balbir Singh, linux-mm

> On Fri, 30 Jul 2010 09:12:26 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > > On Sat, 17 Jul 2010 12:16:33 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
> > > 
> > > > This a complete rewrite of the oom killer's badness() heuristic 
> > > 
> > > Any comments here, or are we ready to proceed?
> > > 
> > > Gimme those acked-bys, reviewed-bys and tested-bys, please!
> > 
> > If he continue to resend all of rewrite patch, I continue to refuse them.
> > I explained it multi times.
> 
> There are about 1000 emails on this topic.  Please briefly explain it again.

Major homework are

- make patch series instead unreviewable all in one patch.
- kill oom_score_adj
- write test way and test result

So, I'm pending reviewing until finish them. I'd like to point out 
rest minor topics while reviewing process.

Thanks.


--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-07-30 11:02       ` KOSAKI Motohiro
@ 2010-07-30 20:14         ` David Rientjes
  2010-08-02 20:43         ` Andrew Morton
  1 sibling, 0 replies; 28+ messages in thread
From: David Rientjes @ 2010-07-30 20:14 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Andrew Morton, Nick Piggin, KAMEZAWA Hiroyuki, Oleg Nesterov,
	Balbir Singh, linux-mm

On Fri, 30 Jul 2010, KOSAKI Motohiro wrote:

> Major homework are
> 
> - make patch series instead unreviewable all in one patch.

Hi, KOSAKI.

We've talked about this point many times.  This particular patch simply 
cannot be split up into multiple patches without dropping existing 
behavior without forward-looking statements that they're going to be 
readded later.  For example, I cannot remove the heuristic that lowers the 
score for CAP_SYS_ADMIN tasks from the current implementation and then say 
"it will be readded in a later patch."  That makes it much more difficult 
to review since it takes a lot more effort to ensure nothing was dropped 
that we still need.  As a consequence, oom_badness() needs to be rewritten 
in its entirety (although the majority of changes are deleting lines from 
the current implementation :) and there are consequences in other areas of 
code like changing function signatures and passing the necessary 
information that the heuristic now uses: the total amount of memory that 
the application is bound to, depending on the context in which the oom 
killer was called.

I've said all of that many times so perhaps it was a misunderstanding 
before and now it's more clear after this iteration.  I don't know.  But 
I'd prefer that if the maintainer, Andrew, disagrees with the methodology 
used to generate this patche (and has said he accepts total rewrites in 
the past, even in the discussion regarding this one), that he disagree 
with the paragraph above.  Otherwise we end up talking in circles.

I've made great efforts to make this rewrite happen because it 
significantly improves the oom killer's behavior on the desktop as well as 
enables us to have a much more powerful tunable for server systems to 
prioritize oom killing.  For example, I dropped the forkbomb detector in 
this iteration since it was pretty controversial.  I'd appreciate it if 
you could take the time to review the patch.

> - kill oom_score_adj

I don't quite understand where this is coming from since there's no 
reasoning given, but oom_score_adj is vital to the ability of userspace to 
tune the new heuristic's baseline.  It is the first time we've ever had 
the ability to tune the oom killing priority of a task based on an actual 
unit.  oom_adj does not work with the new heuristic, which ranks tasks by 
a proportion of resident memory to allowed memory.  That ranking is 
necessary because oom killing priority only makes sense when considered 
relative to other candidate tasks: we want to kill the memory hogging task 
and we don't want to reset oom_adj anytime that task is attached to a 
memcg, a cpuset, or a mempolicy.  It's unreasonable to expect userspace to 
tune oom_adj whenever its attachment to a memcg, a cpuset, or a mempolicy 
changes, and whenever their traits changes: the memcg limit changes, the 
set of allowed cpuset mems changes, or the set of allowed mempolicy nodes 
changes.

> - write test way and test result
> 

I've stated multiple times, and the example is mentioned in the changelog, 
that this change prefers to kill memory hogging tasks over X on a desktop 
system as the result of these changes.  The remainder of the change is 
enabling a more powerful interface for server systems that we need to 
introduce with the new heuristic since oom_adj is obsoleted in that case.  
Perhaps we both don't face the same challenges when it comes to tuning the 
oom killer, so you don't fully understand the problem that I'm addressing 
here.  I've stated the importance of oom_score_adj above, I hope you would 
understand that other people use the oom killer for different reasons 
other than your own and we need this interface.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-07-30 11:02       ` KOSAKI Motohiro
  2010-07-30 20:14         ` David Rientjes
@ 2010-08-02 20:43         ` Andrew Morton
  2010-08-03  0:00           ` KAMEZAWA Hiroyuki
  2010-08-03  6:00           ` KOSAKI Motohiro
  1 sibling, 2 replies; 28+ messages in thread
From: Andrew Morton @ 2010-08-02 20:43 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: David Rientjes, Nick Piggin, KAMEZAWA Hiroyuki, Oleg Nesterov,
	Balbir Singh, linux-mm

On Fri, 30 Jul 2010 20:02:13 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> > On Fri, 30 Jul 2010 09:12:26 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> > 
> > > > On Sat, 17 Jul 2010 12:16:33 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
> > > > 
> > > > > This a complete rewrite of the oom killer's badness() heuristic 
> > > > 
> > > > Any comments here, or are we ready to proceed?
> > > > 
> > > > Gimme those acked-bys, reviewed-bys and tested-bys, please!
> > > 
> > > If he continue to resend all of rewrite patch, I continue to refuse them.
> > > I explained it multi times.
> > 
> > There are about 1000 emails on this topic.  Please briefly explain it again.
> 
> Major homework are
> 
> - make patch series instead unreviewable all in one patch.

Sometimes that's not very practical and the splitup isn't necessarily a
lot easier to understand and review.

It's still possible to review the end result - just read the patched code.

> - kill oom_score_adj

Unclear why?

> - write test way and test result

I think David's done quite a bit of that?

> So, I'm pending reviewing until finish them. I'd like to point out 
> rest minor topics while reviewing process.

I think I'll merge it into 2.6.36.  That gives us two months to
continue to review it, to test it and if necessary, to fix it or revert
it.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-02 20:43         ` Andrew Morton
@ 2010-08-03  0:00           ` KAMEZAWA Hiroyuki
  2010-08-03  0:27             ` David Rientjes
  2010-08-03  6:00           ` KOSAKI Motohiro
  1 sibling, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  0:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: KOSAKI Motohiro, David Rientjes, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Mon, 2 Aug 2010 13:43:12 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 30 Jul 2010 20:02:13 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > > On Fri, 30 Jul 2010 09:12:26 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> > > 
> > > > > On Sat, 17 Jul 2010 12:16:33 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
> > > > > 
> > > > > > This a complete rewrite of the oom killer's badness() heuristic 
> > > > > 
> > > > > Any comments here, or are we ready to proceed?
> > > > > 
> > > > > Gimme those acked-bys, reviewed-bys and tested-bys, please!
> > > > 
> > > > If he continue to resend all of rewrite patch, I continue to refuse them.
> > > > I explained it multi times.
> > > 
> > > There are about 1000 emails on this topic.  Please briefly explain it again.
> > 
> > Major homework are
> > 
> > - make patch series instead unreviewable all in one patch.
> 
> Sometimes that's not very practical and the splitup isn't necessarily a
> lot easier to understand and review.
> 
> It's still possible to review the end result - just read the patched code.
> 
> > - kill oom_score_adj
> 
> Unclear why?
> 

One reason I poitned out is that this new parameter is hard to use for admins and
library writers. 
  old oom_adj was defined as an parameter works as 
		(memory usage of app)/oom_adj.
  new oom_score_adj was define as
		(memory usage of app * oom_score_adj)/ system_memory

Then, an applications' oom_score on a host is quite different from on the other
host. This operation is very new rather than a simple interface updates.
This opinion was rejected.

Anyway, I believe the value other than OOM_DISABLE is useless,
I have no concerns. I'll use memcg if I want to control this kind of things.

Because I know the new calculation logic works better at default, I welcome
this patch itself in general.

Thanks,
-Kame








--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  0:00           ` KAMEZAWA Hiroyuki
@ 2010-08-03  0:27             ` David Rientjes
  2010-08-03  0:36               ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 28+ messages in thread
From: David Rientjes @ 2010-08-03  0:27 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> One reason I poitned out is that this new parameter is hard to use for admins and
> library writers. 
>   old oom_adj was defined as an parameter works as 
> 		(memory usage of app)/oom_adj.

Where are you getting this definition from?

Disregarding all the other small adjustments in the old heuristic, a 
reduced version of the formula was mm->total_vm << oom_adj.  It's a 
shift, not a divide.  That has no sensible meaning.

>   new oom_score_adj was define as
> 		(memory usage of app * oom_score_adj)/ system_memory
> 

No, it's (rss + swap + oom_score_adj) / bound memory.  It's an addition, 
not a multiplication, and it's a proportion of memory the application is 
bound to, not the entire system (it could be constrained by cpuset, 
mempolicy, or memcg).

> Then, an applications' oom_score on a host is quite different from on the other
> host. This operation is very new rather than a simple interface updates.
> This opinion was rejected.
> 

It wasn't rejected, I responded to your comment and you never wrote back.  
The idea 

> Anyway, I believe the value other than OOM_DISABLE is useless,

You're right in that OOM_DISABLE fulfills may typical use cases to simply 
protect a task by making it immune to the oom killer.  But there are other 
use cases for the oom killer that you're perhaps not using where a 
sensible userspace tunable does make a difference: the goal of the 
heuristic is always to kill the task consuming the most amount of memory 
to avoid killing tons of applications for subsequent page allocations.  We 
do run important tasks that consume lots of memory, though, and the kernel 
can't possibly know about that importance.  So although you may never use 
a positive oom_score_adj, although others will, you probably can find a 
use case for subtracting a memory quantity from a known memory hogging 
task that you consider to be vital in an effort to disregard that quantity 
from the score.  I'm sure you'll agree it's a much more powerful (and 
fine-grained) interface than oom_adj.

> I have no concerns. I'll use memcg if I want to control this kind of things.
> 

That would work if you want to setup individual memcgs for every 
application on your system, know what sane limits are for each one, and 
want to incur the significant memory expense of enabling 
CONFIG_CGROUP_MEM_RES_CTLR for its metadata.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  0:27             ` David Rientjes
@ 2010-08-03  0:36               ` KAMEZAWA Hiroyuki
  2010-08-03  1:02                 ` David Rientjes
  0 siblings, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  0:36 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Mon, 2 Aug 2010 17:27:13 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:
> 
> > One reason I poitned out is that this new parameter is hard to use for admins and
> > library writers. 
> >   old oom_adj was defined as an parameter works as 
> > 		(memory usage of app)/oom_adj.
> 
> Where are you getting this definition from?
> 
> Disregarding all the other small adjustments in the old heuristic, a 
> reduced version of the formula was mm->total_vm << oom_adj.  It's a 
> shift, not a divide.  That has no sensible meaning.
> 
yes. that was quite useless.



> >   new oom_score_adj was define as
> > 		(memory usage of app * oom_score_adj)/ system_memory
> > 
> 
> No, it's (rss + swap + oom_score_adj) / bound memory.  It's an addition, 
> not a multiplication, and it's a proportion of memory the application is 
> bound to, not the entire system (it could be constrained by cpuset, 
> mempolicy, or memcg).
> 
sorry.

> > Then, an applications' oom_score on a host is quite different from on the other
> > host. This operation is very new rather than a simple interface updates.
> > This opinion was rejected.
> > 
> 
> It wasn't rejected, I responded to your comment and you never wrote back.  
> The idea 
> 
I just got tired to write the same thing in many times. And I don't have
strong opinions. I _know_ your patch fixes X-server problem. That was enough
for me.


> > Anyway, I believe the value other than OOM_DISABLE is useless,
> 
> You're right in that OOM_DISABLE fulfills may typical use cases to simply 
> protect a task by making it immune to the oom killer.  But there are other 
> use cases for the oom killer that you're perhaps not using where a 
> sensible userspace tunable does make a difference: the goal of the 
> heuristic is always to kill the task consuming the most amount of memory 
> to avoid killing tons of applications for subsequent page allocations.  We 
> do run important tasks that consume lots of memory, though, and the kernel 
> can't possibly know about that importance.  So although you may never use 
> a positive oom_score_adj, although others will, you probably can find a 
> use case for subtracting a memory quantity from a known memory hogging 
> task that you consider to be vital in an effort to disregard that quantity 
> from the score.  I'm sure you'll agree it's a much more powerful (and 
> fine-grained) interface than oom_adj.
> 
Yes, I agree if we can assume the admins are very clever.

> > I have no concerns. I'll use memcg if I want to control this kind of things.
> > 
> 
> That would work if you want to setup individual memcgs for every 
> application on your system, know what sane limits are for each one, and 
> want to incur the significant memory expense of enabling 
> CONFIG_CGROUP_MEM_RES_CTLR for its metadata.
> 
Usual disto alreay enables it.

Simply puts all applications to a group and disable oom and set oom_notifier. 
Then,
 - a "pop-up window" of task list will ask the user "which one do you want to kill ?"
 - send a packet to ask a administlation server system "which one is killable ?"
   or "increase memory limit" or "memory hot-add ?" 

Possible case will be
   - send SIGSTOP to all apps at OOM.
   - rise limit to some extent. or move a killable one to a special group.
   - wake up a killable one with SIGCONT.
   - send SIGHUP to stop it safely.

"My application is killed by the system!!, without running safe emeregency code!"
is the fundamental seeds of disconent.


Thanks,
-Kame


--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  0:36               ` KAMEZAWA Hiroyuki
@ 2010-08-03  1:02                 ` David Rientjes
  2010-08-03  1:08                   ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 28+ messages in thread
From: David Rientjes @ 2010-08-03  1:02 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> > > Then, an applications' oom_score on a host is quite different from on the other
> > > host. This operation is very new rather than a simple interface updates.
> > > This opinion was rejected.
> > > 
> > 
> > It wasn't rejected, I responded to your comment and you never wrote back.  
> > The idea 
> > 
> I just got tired to write the same thing in many times. And I don't have
> strong opinions. I _know_ your patch fixes X-server problem. That was enough
> for me.
> 

There're a couple of reasons why I disagree that oom_score_adj should have 
memory quantity units.

First, individual oom scores that come out of oom_badness() don't mean 
anything in isolation, they only mean something when compared to other 
candidate tasks.  All applications, whether attached to a cpuset, a 
mempolicy, a memcg, or not, have an allowed set of memory and applications 
that are competing for those shared resources.  When defining what 
application happens to be the most memory hogging, which is the one we 
want to kill, they are ranked amongst themselves.  Using oom_score_adj as 
a proportion, we can say a particular application should be allowed 25% of 
resources, other applications should be allowed 5%, and others should be 
penalized 10%, for example.  This makes prioritization for oom kill rather 
simple.

Second, we don't want to adjust oom_score_adj anytime a task is attached 
to a cpuset, a mempolicy, or a memcg, or whenever those cpuset's mems 
changes, the bound mempolicy nodemask changes, or the memcg limit changes.  
The application need not know what that set of allowed memory is and the 
kernel should operate seemlessly regardless of what the attachment is.  
These are, in a sense, "virtualized" systems unto themselves: if a task is 
moved from a child cpuset to the root cpuset, it's set of allowed memory 
may become much larger.  That action shouldn't need to have an equivalent 
change to /proc/pid/oom_score_adj: the priority of the task relative to 
its other competing tasks is the same.  That set of allowed memory may 
change, but its priority does not unless explicitly changed by the admin.

> > That would work if you want to setup individual memcgs for every 
> > application on your system, know what sane limits are for each one, and 
> > want to incur the significant memory expense of enabling 
> > CONFIG_CGROUP_MEM_RES_CTLR for its metadata.
> > 
> Usual disto alreay enables it.
> 

Yes, I'm well aware of my 40MB of lost memory on my laptop :)

> Simply puts all applications to a group and disable oom and set oom_notifier. 
> Then,
>  - a "pop-up window" of task list will ask the user "which one do you want to kill ?"
>  - send a packet to ask a administlation server system "which one is killable ?"
>    or "increase memory limit" or "memory hot-add ?" 
> 

Having user interaction at the time of oom would certainly be nice, but is 
certainly impractical for us.  So we need some way to state the relative 
importance of a task to the kernel so that it can act on our behalf when 
we encounter such a condition.  I believe oom_score_adj does that quite 
effectively.

> Possible case will be
>    - send SIGSTOP to all apps at OOM.
>    - rise limit to some extent. or move a killable one to a special group.
>    - wake up a killable one with SIGCONT.
>    - send SIGHUP to stop it safely.
> 

We use oom notifiers with cpusets, which in this case can be used 
identically to how you're imagining memcg can be used.  This particular 
change, however, only affects the oom killer: that is, it's only scope is 
that when the kernel can't do anything else, no userspace notifier is 
attached, and no memory freeing is going to otherwise occur.  I would love 
to see a per-cgroup oom notifier to allow userspace to respond to these 
conditions in more effective ways, but I still believe there is a general 
need for a simple and predictable oom killer heuristic that the user has 
full power over.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  1:02                 ` David Rientjes
@ 2010-08-03  1:08                   ` KAMEZAWA Hiroyuki
  2010-08-03  1:24                     ` KAMEZAWA Hiroyuki
  2010-08-03  1:50                     ` David Rientjes
  0 siblings, 2 replies; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  1:08 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Mon, 2 Aug 2010 18:02:48 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:
> 
> > > > Then, an applications' oom_score on a host is quite different from on the other
> > > > host. This operation is very new rather than a simple interface updates.
> > > > This opinion was rejected.
> > > > 
> > > 
> > > It wasn't rejected, I responded to your comment and you never wrote back.  
> > > The idea 
> > > 
> > I just got tired to write the same thing in many times. And I don't have
> > strong opinions. I _know_ your patch fixes X-server problem. That was enough
> > for me.
> > 
> 
> There're a couple of reasons why I disagree that oom_score_adj should have 
> memory quantity units.
> 
> First, individual oom scores that come out of oom_badness() don't mean 
> anything in isolation, they only mean something when compared to other 
> candidate tasks.  All applications, whether attached to a cpuset, a 
> mempolicy, a memcg, or not, have an allowed set of memory and applications 
> that are competing for those shared resources.  When defining what 
> application happens to be the most memory hogging, which is the one we 
> want to kill, they are ranked amongst themselves.  Using oom_score_adj as 
> a proportion, we can say a particular application should be allowed 25% of 
> resources, other applications should be allowed 5%, and others should be 
> penalized 10%, for example.  This makes prioritization for oom kill rather 
> simple.
> 
> Second, we don't want to adjust oom_score_adj anytime a task is attached 
> to a cpuset, a mempolicy, or a memcg, or whenever those cpuset's mems 
> changes, the bound mempolicy nodemask changes, or the memcg limit changes.  
> The application need not know what that set of allowed memory is and the 
> kernel should operate seemlessly regardless of what the attachment is.  
> These are, in a sense, "virtualized" systems unto themselves: if a task is 
> moved from a child cpuset to the root cpuset, it's set of allowed memory 
> may become much larger.  That action shouldn't need to have an equivalent 
> change to /proc/pid/oom_score_adj: the priority of the task relative to 
> its other competing tasks is the same.  That set of allowed memory may 
> change, but its priority does not unless explicitly changed by the admin.
> 

Hmm, then, oom_score shows the values for all limitations in array ?


> > > That would work if you want to setup individual memcgs for every 
> > > application on your system, know what sane limits are for each one, and 
> > > want to incur the significant memory expense of enabling 
> > > CONFIG_CGROUP_MEM_RES_CTLR for its metadata.
> > > 
> > Usual disto alreay enables it.
> > 
> 
> Yes, I'm well aware of my 40MB of lost memory on my laptop :)
> 
Very sorry ;)
But it's required to track memory usage from init...

> > Simply puts all applications to a group and disable oom and set oom_notifier. 
> > Then,
> >  - a "pop-up window" of task list will ask the user "which one do you want to kill ?"
> >  - send a packet to ask a administlation server system "which one is killable ?"
> >    or "increase memory limit" or "memory hot-add ?" 
> > 
> 
> Having user interaction at the time of oom would certainly be nice, but is 
> certainly impractical for us.  So we need some way to state the relative 
> importance of a task to the kernel so that it can act on our behalf when 
> we encounter such a condition.  I believe oom_score_adj does that quite 
> effectively.
> 
I don't disagree we need some way. And please take my words as strong objections.
I repeatedly said "I like the patch". but just had small concerns.
And I already explained why I can ignore my concners.


> > Possible case will be
> >    - send SIGSTOP to all apps at OOM.
> >    - rise limit to some extent. or move a killable one to a special group.
> >    - wake up a killable one with SIGCONT.
> >    - send SIGHUP to stop it safely.
> > 
> 
> We use oom notifiers with cpusets, which in this case can be used 
> identically to how you're imagining memcg can be used.  This particular 
> change, however, only affects the oom killer: that is, it's only scope is 
> that when the kernel can't do anything else, no userspace notifier is 
> attached, and no memory freeing is going to otherwise occur.  I would love 
> to see a per-cgroup oom notifier to allow userspace to respond to these 
> conditions in more effective ways, but I still believe there is a general 
> need for a simple and predictable oom killer heuristic that the user has 
> full power over.
> 


yes. the kernel's oom killer should work as the final back-up.
And your new one works very well for X-server case which was an issue for
long time.

Thanks,
-Kame

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  1:08                   ` KAMEZAWA Hiroyuki
@ 2010-08-03  1:24                     ` KAMEZAWA Hiroyuki
  2010-08-03  1:52                       ` David Rientjes
  2010-08-03  1:50                     ` David Rientjes
  1 sibling, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  1:24 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: David Rientjes, Andrew Morton, KOSAKI Motohiro, Nick Piggin,
	Oleg Nesterov, Balbir Singh, linux-mm

On Tue, 3 Aug 2010 10:08:15 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Mon, 2 Aug 2010 18:02:48 -0700 (PDT)
> David Rientjes <rientjes@google.com> wrote:
> 
> > On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:
> > 
> > > > > Then, an applications' oom_score on a host is quite different from on the other
> > > > > host. This operation is very new rather than a simple interface updates.
> > > > > This opinion was rejected.
> > > > > 
> > > > 
> > > > It wasn't rejected, I responded to your comment and you never wrote back.  
> > > > The idea 
> > > > 
> > > I just got tired to write the same thing in many times. And I don't have
> > > strong opinions. I _know_ your patch fixes X-server problem. That was enough
> > > for me.
> > > 
> > 
> > There're a couple of reasons why I disagree that oom_score_adj should have 
> > memory quantity units.
> > 
> > First, individual oom scores that come out of oom_badness() don't mean 
> > anything in isolation, they only mean something when compared to other 
> > candidate tasks.  All applications, whether attached to a cpuset, a 
> > mempolicy, a memcg, or not, have an allowed set of memory and applications 
> > that are competing for those shared resources.  When defining what 
> > application happens to be the most memory hogging, which is the one we 
> > want to kill, they are ranked amongst themselves.  Using oom_score_adj as 
> > a proportion, we can say a particular application should be allowed 25% of 
> > resources, other applications should be allowed 5%, and others should be 
> > penalized 10%, for example.  This makes prioritization for oom kill rather 
> > simple.
> > 
> > Second, we don't want to adjust oom_score_adj anytime a task is attached 
> > to a cpuset, a mempolicy, or a memcg, or whenever those cpuset's mems 
> > changes, the bound mempolicy nodemask changes, or the memcg limit changes.  
> > The application need not know what that set of allowed memory is and the 
> > kernel should operate seemlessly regardless of what the attachment is.  
> > These are, in a sense, "virtualized" systems unto themselves: if a task is 
> > moved from a child cpuset to the root cpuset, it's set of allowed memory 
> > may become much larger.  That action shouldn't need to have an equivalent 
> > change to /proc/pid/oom_score_adj: the priority of the task relative to 
> > its other competing tasks is the same.  That set of allowed memory may 
> > change, but its priority does not unless explicitly changed by the admin.
> > 
> 
> Hmm, then, oom_score shows the values for all limitations in array ?
> 
Anyway, the fact "oom_score can be changed by the context of OOM" may
confuse admins. "OMG, why low oom_score application is killed! Shit!"

Please add additional cares for users if we go this way or remove
user visible oom_score file from /proc.

Thanks,
-kame



--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  1:08                   ` KAMEZAWA Hiroyuki
  2010-08-03  1:24                     ` KAMEZAWA Hiroyuki
@ 2010-08-03  1:50                     ` David Rientjes
  2010-08-03  1:50                       ` KAMEZAWA Hiroyuki
  1 sibling, 1 reply; 28+ messages in thread
From: David Rientjes @ 2010-08-03  1:50 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> Hmm, then, oom_score shows the values for all limitations in array ?
> 

/proc/pid/oom_score will change if a task's cpuset, memcg, or mempolicy 
attachment changes or its mems, nodes, or limit changes because it's a 
proportion of available memory.  /proc/pid/oom_score_adj stays constant 
such that the oom killing priority of that task relative to other tasks 
sharing the same constraints and competing for the same memory is the 
same.  The point is that it doesn't matter how much memory a task has 
available, but rather what it's priority is with the tasks that compete 
with it for memory.

You could, of course, do some simple arithmetic to write a memory quantity 
to oom_score_adj if you really wanted to, but that would force the user to 
recalculate the value anytime the task's cpuset, mempolicy, or memcg 
changes.

> > > Usual disto alreay enables it.
> > > 
> > 
> > Yes, I'm well aware of my 40MB of lost memory on my laptop :)
> > 
> Very sorry ;)
> But it's required to track memory usage from init...
> 

Memcg comes with a cost of ~1% of system memory on x86 since
struct page_cgroup is ~1% of a 4K page.  That means if we were to deploy 
memcg on all of our servers and the number of jobs we can run is 
constrained only by memory, it's equivalent to losing ~1% of our servers.  
That, for us, is very large.

This is a different topic entirely, but it's a very significant 
disadvantage and enough that most people who care about oom killing 
prioritization aren't going to wany to incur such an overhead by enabling 
memcg or setting up individual memcg for each and every job, because that 
requires specific knowledge of all those jobs.

I'm not by any means proposing oom_score_adj as being very popular for the 
usual desktop environments :)

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  1:50                     ` David Rientjes
@ 2010-08-03  1:50                       ` KAMEZAWA Hiroyuki
  0 siblings, 0 replies; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  1:50 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Mon, 2 Aug 2010 18:50:11 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:
> 
> > Hmm, then, oom_score shows the values for all limitations in array ?
> > 
> 
> /proc/pid/oom_score will change if a task's cpuset, memcg, or mempolicy 
> attachment changes or its mems, nodes, or limit changes because it's a 
> proportion of available memory.  /proc/pid/oom_score_adj stays constant 
> such that the oom killing priority of that task relative to other tasks 
> sharing the same constraints and competing for the same memory is the 
> same.  The point is that it doesn't matter how much memory a task has 
> available, but rather what it's priority is with the tasks that compete 
> with it for memory.
> 
> You could, of course, do some simple arithmetic to write a memory quantity 
> to oom_score_adj if you really wanted to, but that would force the user to 
> recalculate the value anytime the task's cpuset, mempolicy, or memcg 
> changes.
> 

My concern is this may corrupt LXC and google's oom_adj system,
expecially when cpuset is used.


> > > > Usual disto alreay enables it.
> > > > 
> > > 
> > > Yes, I'm well aware of my 40MB of lost memory on my laptop :)
> > > 
> > Very sorry ;)
> > But it's required to track memory usage from init...
> > 
> 
> Memcg comes with a cost of ~1% of system memory on x86 since
> struct page_cgroup is ~1% of a 4K page.  That means if we were to deploy 
> memcg on all of our servers and the number of jobs we can run is 
> constrained only by memory, it's equivalent to losing ~1% of our servers.  
> That, for us, is very large.
> 
Google can disable it ;)


> This is a different topic entirely, but it's a very significant 
> disadvantage and enough that most people who care about oom killing 
> prioritization aren't going to wany to incur such an overhead by enabling 
> memcg or setting up individual memcg for each and every job, because that 
> requires specific knowledge of all those jobs.
> 
> I'm not by any means proposing oom_score_adj as being very popular for the 
> usual desktop environments :)
> 
libcgroup will help.

Bye.
-Kame

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  1:24                     ` KAMEZAWA Hiroyuki
@ 2010-08-03  1:52                       ` David Rientjes
  2010-08-03  2:05                         ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 28+ messages in thread
From: David Rientjes @ 2010-08-03  1:52 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> > Hmm, then, oom_score shows the values for all limitations in array ?
> > 
> Anyway, the fact "oom_score can be changed by the context of OOM" may
> confuse admins. "OMG, why low oom_score application is killed! Shit!"
> 
> Please add additional cares for users if we go this way or remove
> user visible oom_score file from /proc.
> 

Sure, a task could be killed with a very low /proc/pid/oom_score, but only 
if its cpuset is oom, for example, and it has the highest score of all 
tasks attached to that oom_score.  So /proc/pid/oom_score needs to be 
considered in the context in which the oom occurs: system-wide, cpuset, 
mempolicy, or memcg.  That's unchanged from the old 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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  1:52                       ` David Rientjes
@ 2010-08-03  2:05                         ` KAMEZAWA Hiroyuki
  2010-08-03  3:05                           ` David Rientjes
  0 siblings, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  2:05 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Mon, 2 Aug 2010 18:52:40 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:
> 
> > > Hmm, then, oom_score shows the values for all limitations in array ?
> > > 
> > Anyway, the fact "oom_score can be changed by the context of OOM" may
> > confuse admins. "OMG, why low oom_score application is killed! Shit!"
> > 
> > Please add additional cares for users if we go this way or remove
> > user visible oom_score file from /proc.
> > 
> 
> Sure, a task could be killed with a very low /proc/pid/oom_score, but only 
> if its cpuset is oom, for example, and it has the highest score of all 
> tasks attached to that oom_score.  So /proc/pid/oom_score needs to be 
> considered in the context in which the oom occurs: system-wide, cpuset, 
> mempolicy, or memcg.  That's unchanged from the old oom killer.
> 

unchanged ?

Assume 2 proceses A, B which has oom_score_adj of 300 and 0
And A uses 200M, B uses 1G of memory under 4G system

Under the system. 
	A's socre = (200M *1000)/4G + 300 = 350
	B's score = (1G * 1000)/4G = 250.

In the cpuset, it has 2G of memory.
	A's score = (200M * 1000)/2G + 300 = 400
	B's socre = (1G * 1000)/2G = 500

This priority-inversion don't happen in current system.

I misunderstand ?


Thanks,
-Kame



--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  2:05                         ` KAMEZAWA Hiroyuki
@ 2010-08-03  3:05                           ` David Rientjes
  2010-08-03  3:11                             ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 28+ messages in thread
From: David Rientjes @ 2010-08-03  3:05 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> > Sure, a task could be killed with a very low /proc/pid/oom_score, but only 
> > if its cpuset is oom, for example, and it has the highest score of all 
> > tasks attached to that oom_score.  So /proc/pid/oom_score needs to be 
> > considered in the context in which the oom occurs: system-wide, cpuset, 
> > mempolicy, or memcg.  That's unchanged from the old oom killer.
> > 
> 
> unchanged ?
> 

Oh, I meant the fact that a task with a low oom_score compared to other 
system tasks may be killed because a cpuset is oom, for instance, is 
unchanged because we only kill tasks that are constrained to that cpuset.

> Assume 2 proceses A, B which has oom_score_adj of 300 and 0
> And A uses 200M, B uses 1G of memory under 4G system
> 
> Under the system. 
> 	A's socre = (200M *1000)/4G + 300 = 350
> 	B's score = (1G * 1000)/4G = 250.

Right, A is penalized 30% of system memory and its use is ~5%, resulting 
in a score of 350, or 35%.  B's use is 25%.

> In the cpuset, it has 2G of memory.
> 	A's score = (200M * 1000)/2G + 300 = 400
> 	B's socre = (1G * 1000)/2G = 500
> 
> This priority-inversion don't happen in current system.
> 

Yes, but this is what oom_score_adj is intended to do: an oom_score_adj of 
300 means task A should be penalized 30% of available memory.  A positive 
oom_score_adj typically means "all other competing tasks should be allowed 
30% more memory, cumulatively, compared to this task."  Task A uses ~10% 
of available memory and task B uses 50% of available memory.  That's a 40% 
difference, which is greater than task A's penalization of 30%, so B is 
killed.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  3:05                           ` David Rientjes
@ 2010-08-03  3:11                             ` KAMEZAWA Hiroyuki
  2010-08-03  4:20                               ` David Rientjes
  0 siblings, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  3:11 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Mon, 2 Aug 2010 20:05:18 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:
> 
> > > Sure, a task could be killed with a very low /proc/pid/oom_score, but only 
> > > if its cpuset is oom, for example, and it has the highest score of all 
> > > tasks attached to that oom_score.  So /proc/pid/oom_score needs to be 
> > > considered in the context in which the oom occurs: system-wide, cpuset, 
> > > mempolicy, or memcg.  That's unchanged from the old oom killer.
> > > 
> > 
> > unchanged ?
> > 
> 
> Oh, I meant the fact that a task with a low oom_score compared to other 
> system tasks may be killed because a cpuset is oom, for instance, is 
> unchanged because we only kill tasks that are constrained to that cpuset.
> 
> > Assume 2 proceses A, B which has oom_score_adj of 300 and 0
> > And A uses 200M, B uses 1G of memory under 4G system
> > 
> > Under the system. 
> > 	A's socre = (200M *1000)/4G + 300 = 350
> > 	B's score = (1G * 1000)/4G = 250.
> 
> Right, A is penalized 30% of system memory and its use is ~5%, resulting 
> in a score of 350, or 35%.  B's use is 25%.
> 
> > In the cpuset, it has 2G of memory.
> > 	A's score = (200M * 1000)/2G + 300 = 400
> > 	B's socre = (1G * 1000)/2G = 500
> > 
> > This priority-inversion don't happen in current system.
> > 
> 
> Yes, but this is what oom_score_adj is intended to do: an oom_score_adj of 
> 300 means task A should be penalized 30% of available memory.  A positive 
> oom_score_adj typically means "all other competing tasks should be allowed 
> 30% more memory, cumulatively, compared to this task."  Task A uses ~10% 
> of available memory and task B uses 50% of available memory.  That's a 40% 
> difference, which is greater than task A's penalization of 30%, so B is 
> killed.
>

This will confuse LXC(Linux Container) guys. oom_score is unusable anymore.

Thanks,
-Kame



--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  3:11                             ` KAMEZAWA Hiroyuki
@ 2010-08-03  4:20                               ` David Rientjes
  2010-08-03  4:32                                 ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 28+ messages in thread
From: David Rientjes @ 2010-08-03  4:20 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> > Yes, but this is what oom_score_adj is intended to do: an oom_score_adj of 
> > 300 means task A should be penalized 30% of available memory.  A positive 
> > oom_score_adj typically means "all other competing tasks should be allowed 
> > 30% more memory, cumulatively, compared to this task."  Task A uses ~10% 
> > of available memory and task B uses 50% of available memory.  That's a 40% 
> > difference, which is greater than task A's penalization of 30%, so B is 
> > killed.
> >
> 
> This will confuse LXC(Linux Container) guys. oom_score is unusable anymore.
> 

>From Documentation/filesystems/proc.txt in 2.6.35:

	3.2 /proc/<pid>/oom_score - Display current oom-killer score
	-------------------------------------------------------------

	This file can be used to check the current score used by the 
	oom-killer is for any given <pid>. Use it together with 
	/proc/<pid>/oom_adj to tune which process should be killed in an 
	out-of-memory situation.

That is unchanged with the rewrite.  /proc/pid/oom_score still exports the 
badness() score used by the oom killer to determine which task to kill: 
the highest score will be killed amongst candidate tasks.  The fact that 
the score can be influenced by cpuset, memcg, or mempolicy constraint is 
irrelevant, we cannot assume anything about the badness() heuristic's 
implementation from the score itself.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  4:20                               ` David Rientjes
@ 2010-08-03  4:32                                 ` KAMEZAWA Hiroyuki
  2010-08-03  7:23                                   ` David Rientjes
  0 siblings, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  4:32 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Mon, 2 Aug 2010 21:20:40 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:
> 
> > > Yes, but this is what oom_score_adj is intended to do: an oom_score_adj of 
> > > 300 means task A should be penalized 30% of available memory.  A positive 
> > > oom_score_adj typically means "all other competing tasks should be allowed 
> > > 30% more memory, cumulatively, compared to this task."  Task A uses ~10% 
> > > of available memory and task B uses 50% of available memory.  That's a 40% 
> > > difference, which is greater than task A's penalization of 30%, so B is 
> > > killed.
> > >
> > 
> > This will confuse LXC(Linux Container) guys. oom_score is unusable anymore.
> > 
> 
> From Documentation/filesystems/proc.txt in 2.6.35:
> 
> 	3.2 /proc/<pid>/oom_score - Display current oom-killer score
> 	-------------------------------------------------------------
> 
> 	This file can be used to check the current score used by the 
> 	oom-killer is for any given <pid>. Use it together with 
> 	/proc/<pid>/oom_adj to tune which process should be killed in an 
> 	out-of-memory situation.
> 
> That is unchanged with the rewrite.  /proc/pid/oom_score still exports the 
> badness() score used by the oom killer to determine which task to kill: 
> the highest score will be killed amongst candidate tasks.  The fact that 
> the score can be influenced by cpuset, memcg, or mempolicy constraint is 
> irrelevant, we cannot assume anything about the badness() heuristic's 
> implementation from the score itself.
> 

In old behavior, oom_score order is synchronous both in the system and
container. High-score one will be killed.
IOW, oom_score have worked as oom_score.

But, after the patch,  the user (of LXC at el.) can't trust oom_score. 
Especially with memcg, it just shows a _broken_ value.

And user has to caluculate oom_score by himself as

real_oom_score = (oom_score - oom_score_adj) *
	system_memory/container_memory + oom_score_adj.

I'm wrong ? Anyway, I think you should take care of this issue.
Maybe this breaks google's oom-killer+cpuset system.

Thanks,
-Kame

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-02 20:43         ` Andrew Morton
  2010-08-03  0:00           ` KAMEZAWA Hiroyuki
@ 2010-08-03  6:00           ` KOSAKI Motohiro
  2010-08-03  7:16             ` David Rientjes
  1 sibling, 1 reply; 28+ messages in thread
From: KOSAKI Motohiro @ 2010-08-03  6:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kosaki.motohiro, David Rientjes, Nick Piggin, KAMEZAWA Hiroyuki,
	Oleg Nesterov, Balbir Singh, linux-mm

> On Fri, 30 Jul 2010 20:02:13 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > > On Fri, 30 Jul 2010 09:12:26 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> > > 
> > > > > On Sat, 17 Jul 2010 12:16:33 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
> > > > > 
> > > > > > This a complete rewrite of the oom killer's badness() heuristic 
> > > > > 
> > > > > Any comments here, or are we ready to proceed?
> > > > > 
> > > > > Gimme those acked-bys, reviewed-bys and tested-bys, please!
> > > > 
> > > > If he continue to resend all of rewrite patch, I continue to refuse them.
> > > > I explained it multi times.
> > > 
> > > There are about 1000 emails on this topic.  Please briefly explain it again.
> > 
> > Major homework are
> > 
> > - make patch series instead unreviewable all in one patch.
> 
> Sometimes that's not very practical and the splitup isn't necessarily a
> lot easier to understand and review.

Yes, sometimes. 
But in this case, _I_ am reviewing the patch sometimes and I'd say. Isn't
this enough reason?


> It's still possible to review the end result - just read the patched code.
> 
> > - kill oom_score_adj
> 
> Unclear why?

Summrize here

1. kosaki pointed some technical issue.

Tue,  8 Jun 2010
KOSAKI Motohiro wrote:
> Sorry I can't ack this. again and again, I try to explain why this is wrong
> (hopefully last)
> 
> 1) incompatibility
>    oom_score is one of ABI. then, we can't change this. from enduser view,
>    this change is no merit. In general, an incompatibility is allowed on very
>    limited situation such as that an end-user get much benefit than compatibility.
>    In other word, old style ABI doesn't works fine from end user view.
>    But, in this case, it isn't.
> 
> 2) technically incorrect
>    this math is not correct math. this is not represented "allowed memory".
>    example, 1) this is not accumulated mlocked memory, but it can be freed
>    task kill 2) SHM_LOCKED memory freeablility depend on IPC_RMID did or not.
>    if not, task killing doesn't free SYSV IPC memory.
>    In additon, 3) This normalization doesn't works on asymmetric numa. 
>    total pages and oom are not related almostly. 4) scalability. if the 
>    system 10TB memory, 1 point oom score mean 10GB memory consumption.
>    it seems too rough. generically, a value suppression itself is evil for
>    scalability software.
> 
> Then, we can't merge this our kernel. if your workload really need this,
> we consider following simplest hook instead.
> 
> 	if (badness_hook_fn)
> 		points = badness_hook_fn(p)
> 	else
> 		points = oom_badness(p);
> 
> Please implement your specific oom-score in your hook func.


2. akpm also wrote this

Andrew Morton wrote:
> On Sun, 6 Jun 2010 15:34:54 -0700 (PDT)
> David Rientjes <rientjes@google.com> wrote:
> 
> > This a complete rewrite of the oom killer's badness() heuristic which is
> > used to determine which task to kill in oom conditions.  The goal is to
> > make it as simple and predictable as possible so the results are better
> > understood and we end up killing the task which will lead to the most
> > memory freeing while still respecting the fine-tuning from userspace.
> 
> It's not obvious from this description that then end result is better! 
> Have you any testcases or scenarios which got improved?
> 
> > Instead of basing the heuristic on mm->total_vm for each task, the task's
> > rss and swap space is used instead.  This is a better indication of the
> > amount of memory that will be freeable if the oom killed task is chosen
> > and subsequently exits.
> 
> Again, why should we optimise for the amount of memory which a killing
> will yield (if that's what you mean).  We only need to free enough
> memory to unblock the oom condition then proceed.
> 
> The last thing we want to do is to kill a process which has consumed
> 1000 CPU hours, or which is providing some system-critical service or
> whatever.  Amount-of-memory-freeable is a relatively minor criterion.
> 
> >  This helps specifically in cases where KDE or
> > GNOME is chosen for oom kill on desktop systems instead of a memory
> > hogging task.
> 
> It helps how?  Examples and test cases?
> 
> > The baseline for the heuristic is a proportion of memory that each task is
> > currently using in memory plus swap compared to the amount of "allowable"
> > memory.
> 
> What does "swap" mean?  swapspace includes swap-backed swapcache,
> un-swap-backed swapcache and non-resident swap.  Which of all these is
> being used here and for what reason?
> 
> >  "Allowable," in this sense, means the system-wide resources for
> > unconstrained oom conditions, the set of mempolicy nodes, the mems
> > attached to current's cpuset, or a memory controller's limit.  The
> > proportion is given on a scale of 0 (never kill) to 1000 (always kill),
> > roughly meaning that if a task has a badness() score of 500 that the task
> > consumes approximately 50% of allowable memory resident in RAM or in swap
> > space.
> 
> So is a new aim of this code to also free up swap space?  Confused.
> 
> > The proportion is always relative to the amount of "allowable" memory and
> > not the total amount of RAM systemwide so that mempolicies and cpusets may
> > operate in isolation; they shall not need to know the true size of the
> > machine on which they are running if they are bound to a specific set of
> > nodes or mems, respectively.
> > 
> > Root tasks are given 3% extra memory just like __vm_enough_memory()
> > provides in LSMs.  In the event of two tasks consuming similar amounts of
> > memory, it is generally better to save root's task.
> > 
> > Because of the change in the badness() heuristic's baseline, it is also
> > necessary to introduce a new user interface to tune it.  It's not possible
> > to redefine the meaning of /proc/pid/oom_adj with a new scale since the
> > ABI cannot be changed for backward compatability.  Instead, a new tunable,
> > /proc/pid/oom_score_adj, is added that ranges from -1000 to +1000.  It may
> > be used to polarize the heuristic such that certain tasks are never
> > considered for oom kill while others may always be considered.  The value
> > is added directly into the badness() score so a value of -500, for
> > example, means to discount 50% of its memory consumption in comparison to
> > other tasks either on the system, bound to the mempolicy, in the cpuset,
> > or sharing the same memory controller.
> > 
> > /proc/pid/oom_adj is changed so that its meaning is rescaled into the
> > units used by /proc/pid/oom_score_adj, and vice versa.  Changing one of
> > these per-task tunables will rescale the value of the other to an
> > equivalent meaning.  Although /proc/pid/oom_adj was originally defined as
> > a bitshift on the badness score, it now shares the same linear growth as
> > /proc/pid/oom_score_adj but with different granularity.  This is required
> > so the ABI is not broken with userspace applications and allows oom_adj to
> > be deprecated for future removal.
> 
> It was a mistake to add oom_adj in the first place.  Because it's a
> user-visible knob which us tied to a particular in-kernel
> implementation.  As we're seeing now, the presence of that knob locks
> us into a particular implementation.
> 
> Given that oom_score_adj is just a rescaled version of oom_adj
> (correct?), I guess things haven't got a lot worse on that front as a
> result of these changes.
> 
> 
> General observation regarding the patch description: I'm not seeing a
> lot of reason for merging the patch!  What value does it bring to our
> users?  What problems got solved?
> 
> Some of Kosaki's observations sounded fairly serious so I'll go into
> wait-and-see mode on this patch.


But any issue have not been fixed yet if my understanding is correct.

I didn't wrote "hey! this is still buggy" because the patch is still
unreviewable chaos and I can missed something.



Another summize here, 

1. I pointed out oom_score_adj is too google specific and harmful for
   desktop user.


> > > But oom_score_adj have no benefit form end-uses view. That's problem.
> > > Please consider to make end-user friendly good patch at first.
> > > 
> > 
> > Of course it does, it actually has units whereas oom_adj only grows or 
> > shrinks the badness score exponentially.  oom_score_adj's units are well 
> > understood: on a machine with 4G of memory, 250 means we're trying to 
> > prejudice it by 1G of memory so that can be used by other tasks, -250 
> > means other tasks should be prejudiced by 1G in comparison to this task, 
> > etc.  It's actually quite powerful.
> 
> And, no real user want such power.
> 
> When we consider desktop user case, End-users don't use oom_adj by themself.
> their application are using it.  It mean now oom_adj behave as syscall like
> system interface, unlike kernel knob. application developers also don't 
> need oom_score_adj because application developers don't know end-users 
> machine mem size.
> 
> Then, you will get the change's merit but end users will get the demerit.
> That's out of balance.


2. DavidR answered this.


> > > Of course it does, it actually has units whereas oom_adj only grows or 
> > > shrinks the badness score exponentially.  oom_score_adj's units are well 
> > > understood: on a machine with 4G of memory, 250 means we're trying to 
> > > prejudice it by 1G of memory so that can be used by other tasks, -250 
> > > means other tasks should be prejudiced by 1G in comparison to this task, 
> > > etc.  It's actually quite powerful.
> > 
> > And, no real user want such power.
> > 
> 
> Google does, and I imagine other users will want to be able to normalize 
> each task's memory usage against the others.  It's perfectly legitimate 
> for one task to consume 3G while another consumes 1G and want to select 
> the 1G task to kill.  Setting the 3G task's oom_score_adj value in this 
> case to be -250, for example, depending on the memory capacity of the 
> machine, makes much more sense than influencing it as a bitshift on 
> top of a vastly unpredictable heuristic with oom_adj.  This seems rather 
> trivial to understand.
> 
> > When we consider desktop user case, End-users don't use oom_adj by themself.
> > their application are using it.  It mean now oom_adj behave as syscall like
> > system interface, unlike kernel knob. application developers also don't 
> > need oom_score_adj because application developers don't know end-users 
> > machine mem size.
> > 
> 
> I agree, oom_score_adj isn't targeted to the desktop nor is it targeted to 
> application developers (unless they are setting it to OOM_SCORE_ADJ_MIN to 
> disable oom killing for that task, for example).  It's targeted at 
> sysadmins and daemons that partition a machine to run a number of 
> concurrent jobs.  It's fine to use memcg, for example, to do such 
> partitioning, but memcg can also cause oom conditions with the cgroup.  We 
> want to be able to tell the kernel, through an interface such as this, 
> that one task shouldn't killed because it's expected to use 3G of memory 
> but should be killed when it's using 8G, for example.

I thought he agree to remove desktop regression and back to requirement 
analisys and make much better patches. but It didn't happen. I'm sad.

Although someone think google usecase is most important in the world, _I_
don't think so yet. I still worry about rest almost all user.

I'm complain just resending an unfixed patch set.



> > - write test way and test result
> 
> I think David's done quite a bit of that?

Hmm, you misunderstand my point, probably my last mail was unclear.
I'm sorry.

I didn't say he didn't tested. I'd say, need to confirm test cases
match typical use case. About two month ago, David posted previous 
patch series. he and you talked about this is well tested. but When
I ran, forkbom detection feature of it don't works at all in typical
case. That said, google testing/production enviromnnet is a bit
differenct from other almost world. I'm worry about this.

And again, I'd prefer to ack all of desktop improvemnt and prefer to
nack all of desktop regression. Is there any reason to refuse test
case inspection?



> > So, I'm pending reviewing until finish them. I'd like to point out 
> > rest minor topics while reviewing process.
> 
> I think I'll merge it into 2.6.36.  That gives us two months to
> continue to review it, to test it and if necessary, to fix it or revert
> it.

I have question. Why did you changed your mention? All of your question
were solved? if so, can you please share your conclustion and decision
reason?


While observe this thread, kamezawa-san found another problem in
oom_score_adj, but he seems to prefer to merge rest parts.

So, I would propose minimum oom_score_adj reverting patch here.
I don't worry rest parts so much. because they don't have ABI change.
so we can revert them later if we've found another issue later.

Thanks.



============================================================
Subject: [PATCH] revert oom_score_adj

oom_score_adj bring to a lot of harm than its worth. and It haven't
get any concensus. so revert it.


Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 Documentation/feature-removal-schedule.txt |   25 -----
 Documentation/filesystems/proc.txt         |   97 +++++++-----------
 fs/proc/base.c                             |   99 +-----------------
 include/linux/memcontrol.h                 |    8 --
 include/linux/oom.h                        |   14 +--
 include/linux/sched.h                      |    1 -
 kernel/fork.c                              |    1 -
 mm/memcontrol.c                            |   18 ----
 mm/oom_kill.c                              |  154 +++++++++++-----------------
 9 files changed, 99 insertions(+), 318 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 19b0a3a..702a5a8 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -151,31 +151,6 @@ Who:	Eric Biederman <ebiederm@xmission.com>
 
 ---------------------------
 
-What:	/proc/<pid>/oom_adj
-When:	August 2012
-Why:	/proc/<pid>/oom_adj allows userspace to influence the oom killer's
-	badness heuristic used to determine which task to kill when the kernel
-	is out of memory.
-
-	The badness heuristic has since been rewritten since the introduction of
-	this tunable such that its meaning is deprecated.  The value was
-	implemented as a bitshift on a score generated by the badness()
-	function that did not have any precise units of measure.  With the
-	rewrite, the score is given as a proportion of available memory to the
-	task allocating pages, so using a bitshift which grows the score
-	exponentially is, thus, impossible to tune with fine granularity.
-
-	A much more powerful interface, /proc/<pid>/oom_score_adj, was
-	introduced with the oom killer rewrite that allows users to increase or
-	decrease the badness() score linearly.  This interface will replace
-	/proc/<pid>/oom_adj.
-
-	A warning will be emitted to the kernel log if an application uses this
-	deprecated interface.  After it is printed once, future warnings will be
-	suppressed until the kernel is rebooted.
-
----------------------------
-
 What:	remove EXPORT_SYMBOL(kernel_thread)
 When:	August 2006
 Files:	arch/*/kernel/*_ksyms.c
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index bf6ab27..9fb6cbe 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -33,8 +33,7 @@ Table of Contents
   2	Modifying System Parameters
 
   3	Per-Process Parameters
-  3.1	/proc/<pid>/oom_adj & /proc/<pid>/oom_score_adj - Adjust the oom-killer
-								score
+  3.1	/proc/<pid>/oom_adj - Adjust the oom-killer score
   3.2	/proc/<pid>/oom_score - Display current oom-killer score
   3.3	/proc/<pid>/io - Display the IO accounting fields
   3.4	/proc/<pid>/coredump_filter - Core dump filtering settings
@@ -1235,64 +1234,42 @@ of the kernel.
 CHAPTER 3: PER-PROCESS PARAMETERS
 ------------------------------------------------------------------------------
 
-3.1 /proc/<pid>/oom_adj & /proc/<pid>/oom_score_adj- Adjust the oom-killer score
---------------------------------------------------------------------------------
-
-These file can be used to adjust the badness heuristic used to select which
-process gets killed in out of memory conditions.
-
-The badness heuristic assigns a value to each candidate task ranging from 0
-(never kill) to 1000 (always kill) to determine which process is targeted.  The
-units are roughly a proportion along that range of allowed memory the process
-may allocate from based on an estimation of its current memory and swap use.
-For example, if a task is using all allowed memory, its badness score will be
-1000.  If it is using half of its allowed memory, its score will be 500.
-
-There is an additional factor included in the badness score: root
-processes are given 3% extra memory over other tasks.
-
-The amount of "allowed" memory depends on the context in which the oom killer
-was called.  If it is due to the memory assigned to the allocating task's cpuset
-being exhausted, the allowed memory represents the set of mems assigned to that
-cpuset.  If it is due to a mempolicy's node(s) being exhausted, the allowed
-memory represents the set of mempolicy nodes.  If it is due to a memory
-limit (or swap limit) being reached, the allowed memory is that configured
-limit.  Finally, if it is due to the entire system being out of memory, the
-allowed memory represents all allocatable resources.
-
-The value of /proc/<pid>/oom_score_adj is added to the badness score before it
-is used to determine which task to kill.  Acceptable values range from -1000
-(OOM_SCORE_ADJ_MIN) to +1000 (OOM_SCORE_ADJ_MAX).  This allows userspace to
-polarize the preference for oom killing either by always preferring a certain
-task or completely disabling it.  The lowest possible value, -1000, is
-equivalent to disabling oom killing entirely for that task since it will always
-report a badness score of 0.
-
-Consequently, it is very simple for userspace to define the amount of memory to
-consider for each task.  Setting a /proc/<pid>/oom_score_adj value of +500, for
-example, is roughly equivalent to allowing the remainder of tasks sharing the
-same system, cpuset, mempolicy, or memory controller resources to use at least
-50% more memory.  A value of -500, on the other hand, would be roughly
-equivalent to discounting 50% of the task's allowed memory from being considered
-as scoring against the task.
-
-For backwards compatibility with previous kernels, /proc/<pid>/oom_adj may also
-be used to tune the badness score.  Its acceptable values range from -16
-(OOM_ADJUST_MIN) to +15 (OOM_ADJUST_MAX) and a special value of -17
-(OOM_DISABLE) to disable oom killing entirely for that task.  Its value is
-scaled linearly with /proc/<pid>/oom_score_adj.
-
-Writing to /proc/<pid>/oom_score_adj or /proc/<pid>/oom_adj will change the
-other with its scaled value.
-
-NOTICE: /proc/<pid>/oom_adj is deprecated and will be removed, please see
-Documentation/feature-removal-schedule.txt.
-
-Caveat: when a parent task is selected, the oom killer will sacrifice any first
-generation children with seperate address spaces instead, if possible.  This
-avoids servers and important system daemons from being killed and loses the
-minimal amount of work.
-
+3.1 /proc/<pid>/oom_adj - Adjust the oom-killer score
+------------------------------------------------------
+
+This file can be used to adjust the score used to select which processes
+should be killed in an  out-of-memory  situation.  Giving it a high score will
+increase the likelihood of this process being killed by the oom-killer.  Valid
+values are in the range -16 to +15, plus the special value -17, which disables
+oom-killing altogether for this process.
+
+The process to be killed in an out-of-memory situation is selected among all others
+based on its badness score. This value equals the original memory size of the process
+and is then updated according to its CPU time (utime + stime) and the
+run time (uptime - start time). The longer it runs the smaller is the score.
+Badness score is divided by the square root of the CPU time and then by
+the double square root of the run time.
+
+Swapped out tasks are killed first. Half of each child's memory size is added to
+the parent's score if they do not share the same memory. Thus forking servers
+are the prime candidates to be killed. Having only one 'hungry' child will make
+parent less preferable than the child.
+
+/proc/<pid>/oom_score shows process' current badness score.
+
+The following heuristics are then applied:
+ * if the task was reniced, its score doubles
+ * superuser or direct hardware access tasks (CAP_SYS_ADMIN, CAP_SYS_RESOURCE
+ 	or CAP_SYS_RAWIO) have their score divided by 4
+ * if oom condition happened in one cpuset and checked process does not belong
+ 	to it, its score is divided by 8
+ * the resulting score is multiplied by two to the power of oom_adj, i.e.
+	points <<= oom_adj when it is positive and
+	points >>= -(oom_adj) otherwise
+
+The task with the highest badness score is then selected and its children
+are killed, process itself will be killed in an OOM situation when it does
+not have children or some of them disabled oom like described above.
 
 3.2 /proc/<pid>/oom_score - Display current oom-killer score
 -------------------------------------------------------------
diff --git a/fs/proc/base.c b/fs/proc/base.c
index cad2e08..f238415 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -434,8 +434,7 @@ static int proc_oom_score(struct task_struct *task, char *buffer)
 
 	read_lock(&tasklist_lock);
 	if (pid_alive(task))
-		points = oom_badness(task, NULL, NULL,
-					totalram_pages + total_swap_pages);
+		points = oom_badness(task, NULL, NULL);
 	read_unlock(&tasklist_lock);
 	return sprintf(buffer, "%lu\n", points);
 }
@@ -1049,24 +1048,8 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf,
 		return -EACCES;
 	}
 
-	/*
-	 * Warn that /proc/pid/oom_adj is deprecated, see
-	 * Documentation/feature-removal-schedule.txt.
-	 */
-	printk_once(KERN_WARNING "%s (%d): /proc/%d/oom_adj is deprecated, "
-			"please use /proc/%d/oom_score_adj instead.\n",
-			current->comm, task_pid_nr(current),
-			task_pid_nr(task), task_pid_nr(task));
 	task->signal->oom_adj = oom_adjust;
-	/*
-	 * Scale /proc/pid/oom_score_adj appropriately ensuring that a maximum
-	 * value is always attainable.
-	 */
-	if (task->signal->oom_adj == OOM_ADJUST_MAX)
-		task->signal->oom_score_adj = OOM_SCORE_ADJ_MAX;
-	else
-		task->signal->oom_score_adj = (oom_adjust * OOM_SCORE_ADJ_MAX) /
-								-OOM_DISABLE;
+
 	unlock_task_sighand(task, &flags);
 	put_task_struct(task);
 
@@ -1079,82 +1062,6 @@ static const struct file_operations proc_oom_adjust_operations = {
 	.llseek		= generic_file_llseek,
 };
 
-static ssize_t oom_score_adj_read(struct file *file, char __user *buf,
-					size_t count, loff_t *ppos)
-{
-	struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode);
-	char buffer[PROC_NUMBUF];
-	int oom_score_adj = OOM_SCORE_ADJ_MIN;
-	unsigned long flags;
-	size_t len;
-
-	if (!task)
-		return -ESRCH;
-	if (lock_task_sighand(task, &flags)) {
-		oom_score_adj = task->signal->oom_score_adj;
-		unlock_task_sighand(task, &flags);
-	}
-	put_task_struct(task);
-	len = snprintf(buffer, sizeof(buffer), "%d\n", oom_score_adj);
-	return simple_read_from_buffer(buf, count, ppos, buffer, len);
-}
-
-static ssize_t oom_score_adj_write(struct file *file, const char __user *buf,
-					size_t count, loff_t *ppos)
-{
-	struct task_struct *task;
-	char buffer[PROC_NUMBUF];
-	unsigned long flags;
-	long oom_score_adj;
-	int err;
-
-	memset(buffer, 0, sizeof(buffer));
-	if (count > sizeof(buffer) - 1)
-		count = sizeof(buffer) - 1;
-	if (copy_from_user(buffer, buf, count))
-		return -EFAULT;
-
-	err = strict_strtol(strstrip(buffer), 0, &oom_score_adj);
-	if (err)
-		return -EINVAL;
-	if (oom_score_adj < OOM_SCORE_ADJ_MIN ||
-			oom_score_adj > OOM_SCORE_ADJ_MAX)
-		return -EINVAL;
-
-	task = get_proc_task(file->f_path.dentry->d_inode);
-	if (!task)
-		return -ESRCH;
-	if (!lock_task_sighand(task, &flags)) {
-		put_task_struct(task);
-		return -ESRCH;
-	}
-	if (oom_score_adj < task->signal->oom_score_adj &&
-			!capable(CAP_SYS_RESOURCE)) {
-		unlock_task_sighand(task, &flags);
-		put_task_struct(task);
-		return -EACCES;
-	}
-
-	task->signal->oom_score_adj = oom_score_adj;
-	/*
-	 * Scale /proc/pid/oom_adj appropriately ensuring that OOM_DISABLE is
-	 * always attainable.
-	 */
-	if (task->signal->oom_score_adj == OOM_SCORE_ADJ_MIN)
-		task->signal->oom_adj = OOM_DISABLE;
-	else
-		task->signal->oom_adj = (oom_score_adj * OOM_ADJUST_MAX) /
-							OOM_SCORE_ADJ_MAX;
-	unlock_task_sighand(task, &flags);
-	put_task_struct(task);
-	return count;
-}
-
-static const struct file_operations proc_oom_score_adj_operations = {
-	.read		= oom_score_adj_read,
-	.write		= oom_score_adj_write,
-};
-
 #ifdef CONFIG_AUDITSYSCALL
 #define TMPBUFLEN 21
 static ssize_t proc_loginuid_read(struct file * file, char __user * buf,
@@ -2727,7 +2634,6 @@ static const struct pid_entry tgid_base_stuff[] = {
 #endif
 	INF("oom_score",  S_IRUGO, proc_oom_score),
 	REG("oom_adj",    S_IRUGO|S_IWUSR, proc_oom_adjust_operations),
-	REG("oom_score_adj", S_IRUGO|S_IWUSR, proc_oom_score_adj_operations),
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
@@ -3062,7 +2968,6 @@ static const struct pid_entry tid_base_stuff[] = {
 #endif
 	INF("oom_score", S_IRUGO, proc_oom_score),
 	REG("oom_adj",   S_IRUGO|S_IWUSR, proc_oom_adjust_operations),
-	REG("oom_score_adj", S_IRUGO|S_IWUSR, proc_oom_score_adj_operations),
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",  S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUSR, proc_sessionid_operations),
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 73564ca..9f1afd3 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -125,8 +125,6 @@ void mem_cgroup_update_file_mapped(struct page *page, int val);
 unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,
 						gfp_t gfp_mask, int nid,
 						int zid);
-u64 mem_cgroup_get_limit(struct mem_cgroup *mem);
-
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct mem_cgroup;
 
@@ -306,12 +304,6 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,
 	return 0;
 }
 
-static inline
-u64 mem_cgroup_get_limit(struct mem_cgroup *mem)
-{
-	return 0;
-}
-
 #endif /* CONFIG_CGROUP_MEM_CONT */
 
 #endif /* _LINUX_MEMCONTROL_H */
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 5e3aa83..9c0d4f0 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -2,9 +2,6 @@
 #define __INCLUDE_LINUX_OOM_H
 
 /*
- * /proc/<pid>/oom_adj is deprecated, see
- * Documentation/feature-removal-schedule.txt.
- *
  * /proc/<pid>/oom_adj set to -17 protects from the oom-killer
  */
 #define OOM_DISABLE (-17)
@@ -12,13 +9,6 @@
 #define OOM_ADJUST_MIN (-16)
 #define OOM_ADJUST_MAX 15
 
-/*
- * /proc/<pid>/oom_score_adj set to OOM_SCORE_ADJ_MIN disables oom killing for
- * pid.
- */
-#define OOM_SCORE_ADJ_MIN	(-1000)
-#define OOM_SCORE_ADJ_MAX	1000
-
 #ifdef __KERNEL__
 
 #include <linux/sched.h>
@@ -40,8 +30,8 @@ enum oom_constraint {
 	CONSTRAINT_MEMCG,
 };
 
-extern unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *mem,
-			const nodemask_t *nodemask, unsigned long totalpages);
+extern unsigned long oom_badness(struct task_struct *p, struct mem_cgroup *mem,
+				const nodemask_t *nodemask);
 extern int try_set_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
 extern void clear_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6276635..f4bcf73 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -627,7 +627,6 @@ struct signal_struct {
 #endif
 
 	int oom_adj;		/* OOM kill score adjustment (bit shift) */
-	int oom_score_adj;	/* OOM kill score adjustment */
 };
 
 /* Context switch must be unlocked if interrupts are to be enabled */
diff --git a/kernel/fork.c b/kernel/fork.c
index 98b4508..a82a65c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -899,7 +899,6 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
 	tty_audit_fork(sig);
 
 	sig->oom_adj = current->signal->oom_adj;
-	sig->oom_score_adj = current->signal->oom_score_adj;
 
 	return 0;
 }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6cf4f1d..2b648ce 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1172,24 +1172,6 @@ static int mem_cgroup_count_children(struct mem_cgroup *mem)
 }
 
 /*
- * Return the memory (and swap, if configured) limit for a memcg.
- */
-u64 mem_cgroup_get_limit(struct mem_cgroup *memcg)
-{
-	u64 limit;
-	u64 memsw;
-
-	limit = res_counter_read_u64(&memcg->res, RES_LIMIT) +
-			total_swap_pages;
-	memsw = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
-	/*
-	 * If memsw is finite and limits the amount of swap space available
-	 * to this memcg, return that limit.
-	 */
-	return min(limit, memsw);
-}
-
-/*
  * Visit the first child (need not be the first child as per the ordering
  * of the cgroup list, since we track last_scanned_child) of @mem and use
  * that to reclaim free pages from.
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 5014e50..c8beaa2 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -32,6 +32,7 @@
 #include <linux/mempolicy.h>
 #include <linux/security.h>
 
+
 int sysctl_panic_on_oom;
 int sysctl_oom_kill_allocating_task;
 int sysctl_oom_dump_tasks = 1;
@@ -143,55 +144,38 @@ static bool oom_unkillable_task(struct task_struct *p, struct mem_cgroup *mem,
 /**
  * oom_badness - heuristic function to determine which candidate task to kill
  * @p: task struct of which task we should calculate
- * @totalpages: total present RAM allowed for page allocation
  *
  * The heuristic for determining which task to kill is made to be as simple and
  * predictable as possible.  The goal is to return the highest value for the
  * task consuming the most memory to avoid subsequent oom failures.
  */
-unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *mem,
-		      const nodemask_t *nodemask, unsigned long totalpages)
+unsigned long oom_badness(struct task_struct *p, struct mem_cgroup *mem,
+			  const nodemask_t *nodemask)
 {
+	int oom_adj = p->signal->oom_adj;
 	int points;
 
 	if (oom_unkillable_task(p, mem, nodemask))
 		return 0;
-
-	p = find_lock_task_mm(p);
-	if (!p)
-		return 0;
-
-	/*
-	 * Shortcut check for OOM_SCORE_ADJ_MIN so the entire heuristic doesn't
-	 * need to be executed for something that cannot be killed.
-	 */
-	if (p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
-		task_unlock(p);
+	if (oom_adj == OOM_DISABLE)
 		return 0;
-	}
 
 	/*
 	 * When the PF_OOM_ORIGIN bit is set, it indicates the task should have
 	 * priority for oom killing.
 	 */
-	if (p->flags & PF_OOM_ORIGIN) {
-		task_unlock(p);
-		return 1000;
-	}
+	if (p->flags & PF_OOM_ORIGIN)
+		return ULONG_MAX;
 
-	/*
-	 * The memory controller may have a limit of 0 bytes, so avoid a divide
-	 * by zero, if necessary.
-	 */
-	if (!totalpages)
-		totalpages = 1;
+	p = find_lock_task_mm(p);
+	if (!p)
+		return 0;
 
 	/*
 	 * The baseline for the badness score is the proportion of RAM that each
 	 * task's rss and swap space use.
 	 */
-	points = (get_mm_rss(p->mm) + get_mm_counter(p->mm, MM_SWAPENTS)) * 1000 /
-			totalpages;
+	points = get_mm_rss(p->mm) + get_mm_counter(p->mm, MM_SWAPENTS);
 	task_unlock(p);
 
 	/*
@@ -202,15 +186,18 @@ unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *mem,
 		points -= 30;
 
 	/*
-	 * /proc/pid/oom_score_adj ranges from -1000 to +1000 such that it may
-	 * either completely disable oom killing or always prefer a certain
-	 * task.
+	 * Adjust the score by oom_adj.
 	 */
-	points += p->signal->oom_score_adj;
+	if (oom_adj) {
+		if (oom_adj > 0) {
+			if (!points)
+				points = 1;
+			points <<= oom_adj;
+		} else
+			points >>= -(oom_adj);
+	}
 
-	if (points < 0)
-		return 0;
-	return (points < 1000) ? points : 1000;
+	return points;
 }
 
 /*
@@ -218,17 +205,11 @@ unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *mem,
  */
 #ifdef CONFIG_NUMA
 static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
-				gfp_t gfp_mask, nodemask_t *nodemask,
-				unsigned long *totalpages)
+					gfp_t gfp_mask, nodemask_t *nodemask)
 {
 	struct zone *zone;
 	struct zoneref *z;
 	enum zone_type high_zoneidx = gfp_zone(gfp_mask);
-	bool cpuset_limited = false;
-	int nid;
-
-	/* Default to all available memory */
-	*totalpages = totalram_pages + total_swap_pages;
 
 	if (!zonelist)
 		return CONSTRAINT_NONE;
@@ -245,33 +226,21 @@ static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
 	 * the page allocator means a mempolicy is in effect.  Cpuset policy
 	 * is enforced in get_page_from_freelist().
 	 */
-	if (nodemask && !nodes_subset(node_states[N_HIGH_MEMORY], *nodemask)) {
-		*totalpages = total_swap_pages;
-		for_each_node_mask(nid, *nodemask)
-			*totalpages += node_spanned_pages(nid);
+	if (nodemask && !nodes_subset(node_states[N_HIGH_MEMORY], *nodemask))
 		return CONSTRAINT_MEMORY_POLICY;
-	}
 
 	/* Check this allocation failure is caused by cpuset's wall function */
 	for_each_zone_zonelist_nodemask(zone, z, zonelist,
 			high_zoneidx, nodemask)
 		if (!cpuset_zone_allowed_softwall(zone, gfp_mask))
-			cpuset_limited = true;
+			return CONSTRAINT_CPUSET;
 
-	if (cpuset_limited) {
-		*totalpages = total_swap_pages;
-		for_each_node_mask(nid, cpuset_current_mems_allowed)
-			*totalpages += node_spanned_pages(nid);
-		return CONSTRAINT_CPUSET;
-	}
 	return CONSTRAINT_NONE;
 }
 #else
 static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
-				gfp_t gfp_mask, nodemask_t *nodemask,
-				unsigned long *totalpages)
+				gfp_t gfp_mask, nodemask_t *nodemask)
 {
-	*totalpages = totalram_pages + total_swap_pages;
 	return CONSTRAINT_NONE;
 }
 #endif
@@ -282,16 +251,16 @@ static enum oom_constraint constrained_alloc(struct zonelist *zonelist,
  *
  * (not docbooked, we don't want this one cluttering up the manual)
  */
-static struct task_struct *select_bad_process(unsigned int *ppoints,
-		unsigned long totalpages, struct mem_cgroup *mem,
-		const nodemask_t *nodemask)
+static struct task_struct *select_bad_process(unsigned long *ppoints,
+		struct mem_cgroup *mem, const nodemask_t *nodemask)
+
 {
 	struct task_struct *p;
 	struct task_struct *chosen = NULL;
 	*ppoints = 0;
 
 	for_each_process(p) {
-		unsigned int points;
+		unsigned long points;
 
 		if (oom_unkillable_task(p, mem, nodemask))
 			continue;
@@ -323,10 +292,10 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
 				return ERR_PTR(-1UL);
 
 			chosen = p;
-			*ppoints = 1000;
+			*ppoints = ULONG_MAX;
 		}
 
-		points = oom_badness(p, mem, nodemask, totalpages);
+		points = oom_badness(p, mem, nodemask);
 		if (points > *ppoints) {
 			chosen = p;
 			*ppoints = points;
@@ -342,7 +311,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
  *
  * Dumps the current memory state of all system tasks, excluding kernel threads.
  * State information includes task's pid, uid, tgid, vm size, rss, cpu, oom_adj
- * value, oom_score_adj value, and name.
+ * value, and name.
  *
  * If the actual is non-NULL, only tasks that are a member of the mem_cgroup are
  * shown.
@@ -354,7 +323,8 @@ static void dump_tasks(const struct mem_cgroup *mem)
 	struct task_struct *p;
 	struct task_struct *task;
 
-	pr_info("[ pid ]   uid  tgid total_vm      rss cpu oom_adj oom_score_adj name\n");
+	printk(KERN_INFO "[ pid ]   uid  tgid total_vm      rss cpu oom_adj "
+	       "name\n");
 	for_each_process(p) {
 		if (p->flags & PF_KTHREAD)
 			continue;
@@ -371,11 +341,10 @@ static void dump_tasks(const struct mem_cgroup *mem)
 			continue;
 		}
 
-		pr_info("[%5d] %5d %5d %8lu %8lu %3u     %3d         %5d %s\n",
-			task->pid, __task_cred(task)->uid, task->tgid,
-			task->mm->total_vm, get_mm_rss(task->mm),
-			task_cpu(task), task->signal->oom_adj,
-			task->signal->oom_score_adj, task->comm);
+		printk(KERN_INFO "[%5d] %5d %5d %8lu %8lu %3u     %3d %s\n",
+		       task->pid, __task_cred(task)->uid, task->tgid,
+		       task->mm->total_vm, get_mm_rss(task->mm),
+		       task_cpu(task), task->signal->oom_adj, task->comm);
 		task_unlock(task);
 	}
 }
@@ -385,9 +354,8 @@ static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order,
 {
 	task_lock(current);
 	pr_warning("%s invoked oom-killer: gfp_mask=0x%x, order=%d, "
-		"oom_adj=%d, oom_score_adj=%d\n",
-		current->comm, gfp_mask, order, current->signal->oom_adj,
-		current->signal->oom_score_adj);
+		   "oom_adj=%d\n",
+		   current->comm, gfp_mask, order, current->signal->oom_adj);
 	cpuset_print_task_mems_allowed(current);
 	task_unlock(current);
 	dump_stack();
@@ -427,14 +395,13 @@ static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
 #undef K
 
 static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
-			    unsigned int points, unsigned long totalpages,
-			    struct mem_cgroup *mem, nodemask_t *nodemask,
-			    const char *message)
+			    unsigned long points, struct mem_cgroup *mem,
+			    nodemask_t *nodemask, const char *message)
 {
 	struct task_struct *victim = p;
 	struct task_struct *child;
 	struct task_struct *t = p;
-	unsigned int victim_points = 0;
+	unsigned long victim_points = 0;
 
 	if (printk_ratelimit())
 		dump_header(p, gfp_mask, order, mem);
@@ -450,7 +417,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 	}
 
 	task_lock(p);
-	pr_err("%s: Kill process %d (%s) score %d or sacrifice child\n",
+	pr_err("%s: Kill process %d (%s) score %lu or sacrifice child\n",
 		message, task_pid_nr(p), p->comm, points);
 	task_unlock(p);
 
@@ -462,13 +429,12 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 	 */
 	do {
 		list_for_each_entry(child, &t->children, sibling) {
-			unsigned int child_points;
+			unsigned long child_points;
 
 			/*
 			 * oom_badness() returns 0 if the thread is unkillable
 			 */
-			child_points = oom_badness(child, mem, nodemask,
-								totalpages);
+			child_points = oom_badness(child, mem, nodemask);
 			if (child_points > victim_points) {
 				victim = child;
 				victim_points = child_points;
@@ -506,19 +472,17 @@ static void check_panic_on_oom(enum oom_constraint constraint, gfp_t gfp_mask,
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR
 void mem_cgroup_out_of_memory(struct mem_cgroup *mem, gfp_t gfp_mask)
 {
-	unsigned long limit;
-	unsigned int points = 0;
+	unsigned long points = 0;
 	struct task_struct *p;
 
 	check_panic_on_oom(CONSTRAINT_MEMCG, gfp_mask, 0);
-	limit = mem_cgroup_get_limit(mem) >> PAGE_SHIFT;
 	read_lock(&tasklist_lock);
 retry:
-	p = select_bad_process(&points, limit, mem, NULL);
+	p = select_bad_process(&points, mem, NULL);
 	if (!p || PTR_ERR(p) == -1UL)
 		goto out;
 
-	if (oom_kill_process(p, gfp_mask, 0, points, limit, mem, NULL,
+	if (oom_kill_process(p, gfp_mask, 0, points, mem, NULL,
 				"Memory cgroup out of memory"))
 		goto retry;
 out:
@@ -643,9 +607,8 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
 		int order, nodemask_t *nodemask)
 {
 	struct task_struct *p;
-	unsigned long totalpages;
 	unsigned long freed = 0;
-	unsigned int points;
+	unsigned long points;
 	enum oom_constraint constraint = CONSTRAINT_NONE;
 
 	blocking_notifier_call_chain(&oom_notify_list, 0, &freed);
@@ -668,8 +631,9 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
 	 * Check if there were limitations on the allocation (only relevant for
 	 * NUMA) that may require different handling.
 	 */
-	constraint = constrained_alloc(zonelist, gfp_mask, nodemask,
-						&totalpages);
+	constraint = constrained_alloc(zonelist, gfp_mask, nodemask);
+	if (constraint != CONSTRAINT_MEMORY_POLICY)
+		nodemask = NULL;
 	check_panic_on_oom(constraint, gfp_mask, order);
 
 	read_lock(&tasklist_lock);
@@ -681,16 +645,14 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
 		 * non-zero, current could not be killed so we must fallback to
 		 * the tasklist scan.
 		 */
-		if (!oom_kill_process(current, gfp_mask, order, 0, totalpages,
-				NULL, nodemask,
+		if (!oom_kill_process(current, gfp_mask, order, 0, NULL,
+				      nodemask,
 				"Out of memory (oom_kill_allocating_task)"))
 			return;
 	}
 
 retry:
-	p = select_bad_process(&points, totalpages, NULL,
-			constraint == CONSTRAINT_MEMORY_POLICY ? nodemask :
-								 NULL);
+	p = select_bad_process(&points, NULL, nodemask);
 	if (PTR_ERR(p) == -1UL)
 		return;
 
@@ -701,8 +663,8 @@ retry:
 		panic("Out of memory and no killable processes...\n");
 	}
 
-	if (oom_kill_process(p, gfp_mask, order, points, totalpages, NULL,
-				nodemask, "Out of memory"))
+	if (oom_kill_process(p, gfp_mask, order, points, NULL, nodemask,
+			     "Out of memory"))
 		goto retry;
 	read_unlock(&tasklist_lock);
 
-- 
1.6.5.2




--
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>

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  6:00           ` KOSAKI Motohiro
@ 2010-08-03  7:16             ` David Rientjes
  0 siblings, 0 replies; 28+ messages in thread
From: David Rientjes @ 2010-08-03  7:16 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Andrew Morton, Nick Piggin, KAMEZAWA Hiroyuki, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KOSAKI Motohiro wrote:

> Tue,  8 Jun 2010
> KOSAKI Motohiro wrote:
> > Sorry I can't ack this. again and again, I try to explain why this is wrong
> > (hopefully last)
> > 
> > 1) incompatibility
> >    oom_score is one of ABI. then, we can't change this. from enduser view,
> >    this change is no merit. In general, an incompatibility is allowed on very
> >    limited situation such as that an end-user get much benefit than compatibility.
> >    In other word, old style ABI doesn't works fine from end user view.
> >    But, in this case, it isn't.
> > 

oom_score is unchanged from its documented purpose, it still reports the 
value that the oom_badness() function returns to decide which task to 
kill, and it's unchanged that the greatest score from the set of allowed 
tasks is the one selected (or its child sacrificed).  The implementation 
of oom_badness() has never been tied directly to the reporting of 
/proc/pid/oom_score.  (With the old heuristic, the score also changed 
based on the cpuset placement and even runtime!)

> > 2) technically incorrect
> >    this math is not correct math. this is not represented "allowed memory".
> >    example, 1) this is not accumulated mlocked memory, but it can be freed
> >    task kill 2) SHM_LOCKED memory freeablility depend on IPC_RMID did or not.
> >    if not, task killing doesn't free SYSV IPC memory.
> >    In additon, 3) This normalization doesn't works on asymmetric numa. 
> >    total pages and oom are not related almostly. 4) scalability. if the 
> >    system 10TB memory, 1 point oom score mean 10GB memory consumption.
> >    it seems too rough. generically, a value suppression itself is evil for
> >    scalability software.
> > 

I responded to this and said that I would change the denominator of the 
fraction from accounting only anonymous and pagecache to all allowed 
memory (totalram_pages, memcg limit, or the aggregate of 
node_spanned_pages).

> Andrew Morton wrote:

I responded directly to this message that akpm wrote and addressed all his 
points.  It went unanswered.  For your convenience, my response is 
archived at http://marc.info/?l=linux-mm&m=127675274612692 from a couple 
months ago.  Why you would quote his email and not my reply is strange.

> Another summize here, 
> 
> 1. I pointed out oom_score_adj is too google specific and harmful for
>    desktop user.
> 

oom_score_adj does nothing if the desktop user doesn't tune it, so I don't 
know what you're referring to here.  The desktop user need not know about 
it.

> I thought he agree to remove desktop regression and back to requirement 
> analisys and make much better patches. but It didn't happen. I'm sad.
> 

There's no desktop regression.

> Although someone think google usecase is most important in the world, _I_
> don't think so yet. I still worry about rest almost all user.
> 

This isn't specific at all to Google, oom_score_adj is a much more 
powerful userspace interface that allows the badness heuristic to be 
influenced in ways that we currently can't.  That's because it actually 
has a unit and works in a predictable and well-defined way.  Arguing that 
we must live with a bitshift on the badness score is not in anyone's best 
interest.

> I didn't say he didn't tested. I'd say, need to confirm test cases
> match typical use case. About two month ago, David posted previous 
> patch series. he and you talked about this is well tested. but When
> I ran, forkbom detection feature of it don't works at all in typical
> case. That said, google testing/production enviromnnet is a bit
> differenct from other almost world. I'm worry about this.
> 

The forkbomb detector was removed because I found it was too controversial 
and I didn't want to hold up making progress on this rewrite which is a 
clear win for both desktop and server users.

> > I think I'll merge it into 2.6.36.  That gives us two months to
> > continue to review it, to test it and if necessary, to fix it or revert
> > it.
> 
> I have question. Why did you changed your mention? All of your question
> were solved? if so, can you please share your conclustion and decision
> reason?
> 

Perhaps he was satisfied with my response to his email that I wrote that 
addressed his concerns either directly or with changes to this most recent 
revision.  The patch that is now merged in -mm is different from previous 
versions: the denominator of the fraction was changed (at your request) 
and the forkbomb detector was removed (at yours and others request).

> So, I would propose minimum oom_score_adj reverting patch here.
> I don't worry rest parts so much. because they don't have ABI change.
> so we can revert them later if we've found another issue later.
> 
> Thanks.
> 
> 
> 
> ============================================================
> Subject: [PATCH] revert oom_score_adj
> 
> oom_score_adj bring to a lot of harm than its worth. and It haven't
> get any concensus. so revert it.
> 

This is becoming very typical, KOSAKI, and it's getting old.  You're 
proposing sweeping changes without any reasoning behind them.  If you have 
a problem with /proc/pid/oom_score_adj, please enumerate them here and now 
and we can discuss them.  Nobody here is interested in looking at old 
revisions of this change, looking through hundreds of emails, or trying to 
infer what you're talking about.  Thanks.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  7:23                                   ` David Rientjes
@ 2010-08-03  7:21                                     ` KAMEZAWA Hiroyuki
  2010-08-03  7:27                                       ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  7:21 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010 00:23:32 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> > Especially with memcg, it just shows a _broken_ value.
> > 
> 
> Not at all, the user knows what tasks are attached to the memcg and can 
> easily determine which task is going to be killed when it ooms: simply 
> iterate through the memcg tasklist, check /proc/pid/oom_score, and sort.
> 

And finds 
	at system oom,  process A is killed.
	at memcg oom, process B is killed.

funny non-deteministic interace, aha.

Thanks,
-Kame


--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  4:32                                 ` KAMEZAWA Hiroyuki
@ 2010-08-03  7:23                                   ` David Rientjes
  2010-08-03  7:21                                     ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 28+ messages in thread
From: David Rientjes @ 2010-08-03  7:23 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> In old behavior, oom_score order is synchronous both in the system and
> container. High-score one will be killed.
> IOW, oom_score have worked as oom_score.
> 

This isn't necessarily true as I've already pointed out: the highest score 
as exported by /proc/pid/oom_score is not always killed if it's not a 
candidate task: it may be in a disjoint memcg, for example.  The highest 
_candidate_ task is killed, and that's unchanged with my rewrite.

The current /proc/pid/oom_score is also not synchronous between the system 
and container at least in the cpuset case since we currently divide a 
task's score by 8 if it doesn't intersect current's mems_allowed, so 
that's not true either.

> But, after the patch,  the user (of LXC at el.) can't trust oom_score. 

Yes, they can, but they need to know the context in which the oom occurs.  
/proc/pid/oom_score cannot export multiple values although its kill 
ranking actually depends on whether its a system oom, memcg oom, cpuset 
oom, etc.  It needs to export a single value as a function of the 
heuristic.  The user must then take those values at the time of 
collection and find how the various tasks rank relative to one another 
depending on MPOL_BIND, cpuset hierarchy, etc.  That's actually not that 
difficult because admins who don't use any cgroups typically only have 
system-wide ooms where oom_score is always accurate and admins who use 
cpusets or memcg or mempolicies on large NUMA systems already know the set 
of tasks that are attached to them and want to prioritize the killing list 
specifically for those entities.

> Especially with memcg, it just shows a _broken_ value.
> 

Not at all, the user knows what tasks are attached to the memcg and can 
easily determine which task is going to be killed when it ooms: simply 
iterate through the memcg tasklist, check /proc/pid/oom_score, and sort.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  7:21                                     ` KAMEZAWA Hiroyuki
@ 2010-08-03  7:27                                       ` KAMEZAWA Hiroyuki
  2010-08-03 20:43                                         ` David Rientjes
  0 siblings, 1 reply; 28+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-08-03  7:27 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: David Rientjes, Andrew Morton, KOSAKI Motohiro, Nick Piggin,
	Oleg Nesterov, Balbir Singh, linux-mm

On Tue, 3 Aug 2010 16:21:11 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Tue, 3 Aug 2010 00:23:32 -0700 (PDT)
> David Rientjes <rientjes@google.com> wrote:
> 
> > > Especially with memcg, it just shows a _broken_ value.
> > > 
> > 
> > Not at all, the user knows what tasks are attached to the memcg and can 
> > easily determine which task is going to be killed when it ooms: simply 
> > iterate through the memcg tasklist, check /proc/pid/oom_score, and sort.
> > 
> 
> And finds 
> 	at system oom,  process A is killed.
> 	at memcg oom, process B is killed.
> 
> funny non-deteministic interace, aha.
> 

I said
	- you have beutiful legs. (new logic of oom calculation)
	- but your face is ugly	  (new oom_score_adj)
	then
	- I bought a mask for you (oom_disable for memcg.)

Then, please don't use your time for convince me your face is beutiful.
It's waste of time.

Thanks,
-Kame


--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch -mm 1/2] oom: badness heuristic rewrite
  2010-08-03  7:27                                       ` KAMEZAWA Hiroyuki
@ 2010-08-03 20:43                                         ` David Rientjes
  0 siblings, 0 replies; 28+ messages in thread
From: David Rientjes @ 2010-08-03 20:43 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, KOSAKI Motohiro, Nick Piggin, Oleg Nesterov,
	Balbir Singh, linux-mm

On Tue, 3 Aug 2010, KAMEZAWA Hiroyuki wrote:

> > > Not at all, the user knows what tasks are attached to the memcg and can 
> > > easily determine which task is going to be killed when it ooms: simply 
> > > iterate through the memcg tasklist, check /proc/pid/oom_score, and sort.
> > > 
> > 
> > And finds 
> > 	at system oom,  process A is killed.
> > 	at memcg oom, process B is killed.
> > 
> > funny non-deteministic interace, aha.
> > 
> 
> I said
> 	- you have beutiful legs. (new logic of oom calculation)
> 	- but your face is ugly	  (new oom_score_adj)
> 	then
> 	- I bought a mask for you (oom_disable for memcg.)
> 
> Then, please don't use your time for convince me your face is beutiful.
> It's waste of time.
> 

Haha, I love the analogy :)

The difference in selection of tasks depending on whether its a memcg or 
system oom isn't really concerning since /proc/pid/oom_score_adj would 
have been assigned in a way that reflects its prioritization for kill only 
in the memcg oom kill scenario.  It would reflect the prioritization 
system wide if memcg were not used.

The same thing would happen if several tasks in different memcgs were 
given oom_score_adj of +1000.  This would yield a badness score of 1000 
for each of those tasks, which translates to "always kill."  Only one 
would actually be selected for kill in the system wide case, however, to 
prevent needless killing.  Which one that is happens to be the one that 
appears in the tasklist first.

The end result is that when using memcg, we don't really care about the 
prioritization of killing of tasks when the entire system is oom.  That's 
true because no memcg happens to be oom itself, we've simply run out of 
RAM.  The same is true of cpusets.

--
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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2010-08-03 20:32 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-17 19:16 [patch -mm 1/2] oom: badness heuristic rewrite David Rientjes
2010-07-17 19:16 ` [patch -mm 2/2] oom: deprecate oom_adj tunable David Rientjes
2010-07-29 23:08 ` [patch -mm 1/2] oom: badness heuristic rewrite Andrew Morton
2010-07-30  0:12   ` KOSAKI Motohiro
2010-07-30  1:38     ` Andrew Morton
2010-07-30 11:02       ` KOSAKI Motohiro
2010-07-30 20:14         ` David Rientjes
2010-08-02 20:43         ` Andrew Morton
2010-08-03  0:00           ` KAMEZAWA Hiroyuki
2010-08-03  0:27             ` David Rientjes
2010-08-03  0:36               ` KAMEZAWA Hiroyuki
2010-08-03  1:02                 ` David Rientjes
2010-08-03  1:08                   ` KAMEZAWA Hiroyuki
2010-08-03  1:24                     ` KAMEZAWA Hiroyuki
2010-08-03  1:52                       ` David Rientjes
2010-08-03  2:05                         ` KAMEZAWA Hiroyuki
2010-08-03  3:05                           ` David Rientjes
2010-08-03  3:11                             ` KAMEZAWA Hiroyuki
2010-08-03  4:20                               ` David Rientjes
2010-08-03  4:32                                 ` KAMEZAWA Hiroyuki
2010-08-03  7:23                                   ` David Rientjes
2010-08-03  7:21                                     ` KAMEZAWA Hiroyuki
2010-08-03  7:27                                       ` KAMEZAWA Hiroyuki
2010-08-03 20:43                                         ` David Rientjes
2010-08-03  1:50                     ` David Rientjes
2010-08-03  1:50                       ` KAMEZAWA Hiroyuki
2010-08-03  6:00           ` KOSAKI Motohiro
2010-08-03  7:16             ` David Rientjes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).