lkml.org 
[lkml]   [2009]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[patch 7/7] oom: prevent possible OOM_DISABLE livelock
    It is currently possible to livelock the oom killer if a task is chosen
    for oom kill and another thread sharing the same memory has an oom_adj
    value of OOM_DISABLE. This occurs because oom_kill_task() repeatedly
    returns 1 and refuses to kill the chosen task while select_bad_process()
    will repeatedly chooses the same task during the next retry.

    This moves the check for OOM_DISABLE to the badness heuristic while
    holding task_lock(). Badness scores of 0 are now explicitly prohibited
    from being oom killed and 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>
    Signed-off-by: David Rientjes <rientjes@google.com>
    ---
    mm/oom_kill.c | 40 ++++++++++------------------------------
    1 files changed, 10 insertions(+), 30 deletions(-)

    diff --git a/mm/oom_kill.c b/mm/oom_kill.c
    --- a/mm/oom_kill.c
    +++ b/mm/oom_kill.c
    @@ -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,13 +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)
    - continue;
    - task_unlock(p);
    -
    points = badness(p, uptime.tv_sec);
    - if (points > *ppoints || !chosen) {
    + if (points > *ppoints) {
    chosen = p;
    *ppoints = points;
    }
    @@ -352,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);

    /*

    \
     
     \ /
      Last update: 2009-05-05 02:33    [W:0.024 / U:18.312 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site