oom: avoid unnecessary mm locking and scanning for OOM_DISABLE
David Rientjes [Tue, 16 Jun 2009 22:32:57 +0000 (15:32 -0700)]
This moves the check for OOM_DISABLE to the badness heuristic so it is
only necessary to hold task_lock() once.  If the mm is OOM_DISABLE, the
score is 0, which is also correctly exported via /proc/pid/oom_score.
This requires that tasks with badness scores of 0 are prohibited from
being oom killed, which makes sense since they would not allow for future
memory freeing anyway.

Since the oom_adj value is a characteristic of an mm and not a task, it is
no longer necessary to check the oom_adj value for threads sharing the
same memory (except when simply issuing SIGKILLs for threads in other
thread groups).

Cc: Nick Piggin <npiggin@suse.de>
Cc: Rik van Riel <riel@redhat.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Signed-off-by: David Rientjes <rientjes@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

mm/oom_kill.c

index b609135..cdcf89c 100644 (file)
@@ -67,6 +67,10 @@ unsigned long badness(struct task_struct *p, unsigned long uptime)
                return 0;
        }
        oom_adj = mm->oom_adj;
+       if (oom_adj == OOM_DISABLE) {
+               task_unlock(p);
+               return 0;
+       }
 
        /*
         * The memory size of the process is the basis for the badness.
@@ -253,15 +257,8 @@ static struct task_struct *select_bad_process(unsigned long *ppoints,
                        *ppoints = ULONG_MAX;
                }
 
-               task_lock(p);
-               if (p->mm && p->mm->oom_adj == OOM_DISABLE) {
-                       task_unlock(p);
-                       continue;
-               }
-               task_unlock(p);
-
                points = badness(p, uptime.tv_sec);
-               if (points > *ppoints || !chosen) {
+               if (points > *ppoints) {
                        chosen = p;
                        *ppoints = points;
                }
@@ -354,32 +351,13 @@ static int oom_kill_task(struct task_struct *p)
        struct mm_struct *mm;
        struct task_struct *g, *q;
 
+       task_lock(p);
        mm = p->mm;
-
-       /* WARNING: mm may not be dereferenced since we did not obtain its
-        * value from get_task_mm(p).  This is OK since all we need to do is
-        * compare mm to q->mm below.
-        *
-        * Furthermore, even if mm contains a non-NULL value, p->mm may
-        * change to NULL at any time since we do not hold task_lock(p).
-        * However, this is of no concern to us.
-        */
-
-       if (mm == NULL)
+       if (!mm || mm->oom_adj == OOM_DISABLE) {
+               task_unlock(p);
                return 1;
-
-       /*
-        * Don't kill the process if any threads are set to OOM_DISABLE
-        */
-       do_each_thread(g, q) {
-               task_lock(q);
-               if (q->mm == mm && q->mm && q->mm->oom_adj == OOM_DISABLE) {
-                       task_unlock(q);
-                       return 1;
-               }
-               task_unlock(q);
-       } while_each_thread(g, q);
-
+       }
+       task_unlock(p);
        __oom_kill_task(p, 1);
 
        /*