21 Aug, 2010

3 commits

  • dump_tasks() needs to hold the RCU read lock around its access of the
    target task's UID. To this end it should use task_uid() as it only needs
    that one thing from the creds.

    The fact that dump_tasks() holds tasklist_lock is insufficient to prevent the
    target process replacing its credentials on another CPU.

    Then, this patch change to call rcu_read_lock() explicitly.

    ===================================================
    [ INFO: suspicious rcu_dereference_check() usage. ]
    ---------------------------------------------------
    mm/oom_kill.c:410 invoked rcu_dereference_check() without protection!

    other info that might help us debug this:

    rcu_scheduler_active = 1, debug_locks = 1
    4 locks held by kworker/1:2/651:
    #0: (events){+.+.+.}, at: []
    process_one_work+0x137/0x4a0
    #1: (moom_work){+.+...}, at: []
    process_one_work+0x137/0x4a0
    #2: (tasklist_lock){.+.+..}, at: []
    out_of_memory+0x164/0x3f0
    #3: (&(&p->alloc_lock)->rlock){+.+...}, at: []
    find_lock_task_mm+0x2e/0x70

    Signed-off-by: KOSAKI Motohiro
    Signed-off-by: David Howells
    Acked-by: Paul E. McKenney
    Acked-by: David Rientjes
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • Commit 0aad4b3124 ("oom: fold __out_of_memory into out_of_memory")
    introduced a tasklist_lock leak. Then it caused following obvious
    danger warnings and panic.

    ================================================
    [ BUG: lock held when returning to user space! ]
    ------------------------------------------------
    rsyslogd/1422 is leaving the kernel with locks still held!
    1 lock held by rsyslogd/1422:
    #0: (tasklist_lock){.+.+.+}, at: [] out_of_memory+0x164/0x3f0
    BUG: scheduling while atomic: rsyslogd/1422/0x00000002
    INFO: lockdep is turned off.

    This patch fixes it.

    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Acked-by: David Rientjes
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • Commit b940fd7035 ("oom: remove unnecessary code and cleanup") added an
    unnecessary NULL pointer dereference. remove it.

    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Acked-by: David Rientjes
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     

11 Aug, 2010

1 commit

  • When the OOM killer scans task, it check a task is under memcg or
    not when it's called via memcg's context.

    But, as Oleg pointed out, a thread group leader may have NULL ->mm
    and task_in_mem_cgroup() may do wrong decision. We have to use
    find_lock_task_mm() in memcg as generic OOM-Killer does.

    Signed-off-by: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Cc: Daisuke Nishimura
    Cc: Balbir Singh
    Reviewed-by: Minchan Kim
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KAMEZAWA Hiroyuki
     

10 Aug, 2010

30 commits

  • 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
    Cc: Nick Piggin
    Cc: KAMEZAWA Hiroyuki
    Cc: KOSAKI Motohiro
    Cc: Oleg Nesterov
    Cc: Balbir Singh
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • Oleg pointed out current PF_EXITING check is wrong. Because PF_EXITING
    is per-thread flag, not per-process flag. He said,

    Two threads, group-leader L and its sub-thread T. T dumps the code.
    In this case both threads have ->mm != NULL, L has PF_EXITING.

    The first problem is, select_bad_process() always return -1 in this
    case (even if the caller is T, this doesn't matter).

    The second problem is that we should add TIF_MEMDIE to T, not L.

    I think we can remove this dubious PF_EXITING check. but as first step,
    This patch add the protection of multi threaded issue.

    Signed-off-by: KOSAKI Motohiro
    Cc: Oleg Nesterov
    Cc: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • In a system under heavy load it was observed that even after the
    oom-killer selects a task to die, the task may take a long time to die.

    Right after sending a SIGKILL to the task selected by the oom-killer this
    task has its priority increased so that it can exit() soon, freeing
    memory. That is accomplished by:

    /*
    * We give our sacrificial lamb high priority and access to
    * all the memory it needs. That way it should be able to
    * exit() and clear out its resources quickly...
    */
    p->rt.time_slice = HZ;
    set_tsk_thread_flag(p, TIF_MEMDIE);

    It sounds plausible giving the dying task an even higher priority to be
    sure it will be scheduled sooner and free the desired memory. It was
    suggested on LKML using SCHED_FIFO:1, the lowest RT priority so that this
    task won't interfere with any running RT task.

    If the dying task is already an RT task, leave it untouched. Another good
    suggestion, implemented here, was to avoid boosting the dying task
    priority in case of mem_cgroup OOM.

    Signed-off-by: Luis Claudio R. Goncalves
    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Luis Claudio R. Goncalves
     
  • The current "child->mm == p->mm" check prevents selection of vfork()ed
    task. But we don't have any reason to don't consider vfork().

    Removed.

    Signed-off-by: KOSAKI Motohiro
    Cc: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • presently has_intersects_mems_allowed() has own thread iterate logic, but
    it should use while_each_thread().

    It slightly improve the code readability.

    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • Presently if oom_kill_allocating_task is enabled and current have
    OOM_DISABLED, following printk in oom_kill_process is called twice.

    pr_err("%s: Kill process %d (%s) score %lu or sacrifice child\n",
    message, task_pid_nr(p), p->comm, points);

    So, OOM_DISABLE check should be more early.

    Signed-off-by: KOSAKI Motohiro
    Cc: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • select_bad_process() and badness() have the same OOM_DISABLE check. This
    patch kills one.

    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • If a kernel thread is using use_mm(), badness() returns a positive value.
    This is not a big issue because caller take care of it correctly. But
    there is one exception, /proc//oom_score calls badness() directly and
    doesn't care that the task is a regular process.

    Another example, /proc/1/oom_score return !0 value. But it's unkillable.
    This incorrectness makes administration a little confusing.

    This patch fixes it.

    Signed-off-by: KOSAKI Motohiro
    Cc: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • When oom_kill_allocating_task is enabled, an argument task of
    oom_kill_process is not selected by select_bad_process(), It's just
    out_of_memory() caller task. It mean the task can be unkillable. check
    it first.

    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • Presently we have the same task check in two places. Unify it.

    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • Presently select_bad_process() has a PF_KTHREAD check, but
    oom_kill_process doesn't. It mean oom_kill_process() may choose wrong
    task, especially, when the child are using use_mm().

    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • Presently, badness() doesn't care about either CPUSET nor mempolicy. Then
    if the victim child process have disjoint nodemask, OOM Killer might kill
    innocent process.

    This patch fixes it.

    [akpm@linux-foundation.org: coding-style fixes]
    Signed-off-by: KOSAKI Motohiro
    Reviewed-by: Minchan Kim
    Cc: Minchan Kim
    Cc: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • __out_of_memory() only has a single caller, so fold it into
    out_of_memory() and add a comment about locking for its call to
    oom_kill_process().

    Signed-off-by: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Acked-by: KOSAKI Motohiro
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • select_bad_process() and __out_of_memory() doe not need their enum
    oom_constraint arguments: it's possible to pass a NULL nodemask if
    constraint == CONSTRAINT_MEMORY_POLICY in the caller, out_of_memory().

    Signed-off-by: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Acked-by: KOSAKI Motohiro
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • We have been used naming try_set_zone_oom and clear_zonelist_oom.
    The role of functions is to lock of zonelist for preventing parallel
    OOM. So clear_zonelist_oom makes sense but try_set_zone_oome is rather
    awkward and unmatched with clear_zonelist_oom.

    Let's change it with try_set_zonelist_oom.

    Signed-off-by: Minchan Kim
    Acked-by: David Rientjes
    Reviewed-by: KOSAKI Motohiro
    Cc: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Minchan Kim
     
  • Remove the redundancy in __oom_kill_task() since:

    - init can never be passed to this function: it will never be PF_EXITING
    or selectable from select_bad_process(), and

    - it will never be passed a task from oom_kill_task() without an ->mm
    and we're unconcerned about detachment from exiting tasks, there's no
    reason to protect them against SIGKILL or access to memory reserves.

    Also moves the kernel log message to a higher level since the verbosity is
    not always emitted here; we need not print an error message if an exiting
    task is given a longer timeslice.

    __oom_kill_task() only has a single caller, so it can be merged into that
    function at the same time.

    Signed-off-by: David Rientjes
    Reviewed-by: KAMEZAWA Hiroyuki
    Cc: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • It is possible to remove the special pagefault oom handler by simply oom
    locking all system zones and then calling directly into out_of_memory().

    All populated zones must have ZONE_OOM_LOCKED set, otherwise there is a
    parallel oom killing in progress that will lead to eventual memory freeing
    so it's not necessary to needlessly kill another task. The context in
    which the pagefault is allocating memory is unknown to the oom killer, so
    this is done on a system-wide level.

    If a task has already been oom killed and hasn't fully exited yet, this
    will be a no-op since select_bad_process() recognizes tasks across the
    system with TIF_MEMDIE set.

    Signed-off-by: David Rientjes
    Acked-by: Nick Piggin
    Acked-by: KOSAKI Motohiro
    Cc: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • There are various points in the oom killer where the kernel must determine
    whether to panic or not. It's better to extract this to a helper function
    to remove all the confusion as to its semantics.

    Also fix a call to dump_header() where tasklist_lock is not read- locked,
    as required.

    There's no functional change with this patch.

    Acked-by: KOSAKI Motohiro
    Signed-off-by: David Rientjes
    Cc: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • The oom killer tasklist dump, enabled with the oom_dump_tasks sysctl, is
    very helpful information in diagnosing why a user's task has been killed.
    It emits useful information such as each eligible thread's memory usage
    that can determine why the system is oom, so it should be enabled by
    default.

    Signed-off-by: David Rientjes
    Acked-by: KOSAKI Motohiro
    Cc: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • The oom killer presently kills current whenever there is no more memory
    free or reclaimable on its mempolicy's nodes. There is no guarantee that
    current is a memory-hogging task or that killing it will free any
    substantial amount of memory, however.

    In such situations, it is better to scan the tasklist for nodes that are
    allowed to allocate on current's set of nodes and kill the task with the
    highest badness() score. This ensures that the most memory-hogging task,
    or the one configured by the user with /proc/pid/oom_adj, is always
    selected in such scenarios.

    Signed-off-by: David Rientjes
    Reviewed-by: KOSAKI Motohiro
    Cc: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • When a task is chosen for oom kill, the oom killer first attempts to
    sacrifice a child not sharing its parent's memory instead. Unfortunately,
    this often kills in a seemingly random fashion based on the ordering of
    the selected task's child list. Additionally, it is not guaranteed at all
    to free a large amount of memory that we need to prevent additional oom
    killing in the very near future.

    Instead, we now only attempt to sacrifice the worst child not sharing its
    parent's memory, if one exists. The worst child is indicated with the
    highest badness() score. This serves two advantages: we kill a
    memory-hogging task more often, and we allow the configurable
    /proc/pid/oom_adj value to be considered as a factor in which child to
    kill.

    Reviewers may observe that the previous implementation would iterate
    through the children and attempt to kill each until one was successful and
    then the parent if none were found while the new code simply kills the
    most memory-hogging task or the parent. Note that the only time
    oom_kill_task() fails, however, is when a child does not have an mm or has
    a /proc/pid/oom_adj of OOM_DISABLE. badness() returns 0 for both cases,
    so the final oom_kill_task() will always succeed.

    Signed-off-by: David Rientjes
    Acked-by: Rik van Riel
    Acked-by: Nick Piggin
    Acked-by: Balbir Singh
    Cc: KOSAKI Motohiro
    Reviewed-by: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • Tasks that do not share the same set of allowed nodes with the task that
    triggered the oom should not be considered as candidates for oom kill.

    Tasks in other cpusets with a disjoint set of mems would be unfairly
    penalized otherwise because of oom conditions elsewhere; an extreme
    example could unfairly kill all other applications on the system if a
    single task in a user's cpuset sets itself to OOM_DISABLE and then uses
    more memory than allowed.

    Killing tasks outside of current's cpuset rarely would free memory for
    current anyway. To use a sane heuristic, we must ensure that killing a
    task would likely free memory for current and avoid needlessly killing
    others at all costs just because their potential memory freeing is
    unknown. It is better to kill current than another task needlessly.

    Signed-off-by: David Rientjes
    Acked-by: Rik van Riel
    Acked-by: Nick Piggin
    Acked-by: Balbir Singh
    Cc: KOSAKI Motohiro
    Reviewed-by: KAMEZAWA Hiroyuki
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • It's unnecessary to SIGKILL a task that is already PF_EXITING and can
    actually cause a NULL pointer dereference of the sighand if it has already
    been detached. Instead, simply set TIF_MEMDIE so it has access to memory
    reserves and can quickly exit as the comment implies.

    Reviewed-by: KAMEZAWA Hiroyuki
    Signed-off-by: David Rientjes
    Cc: KOSAKI Motohiro
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • It's possible to livelock the page allocator if a thread has mm->mmap_sem
    and fails to make forward progress because the oom killer selects another
    thread sharing the same ->mm to kill that cannot exit until the semaphore
    is dropped.

    The oom killer will not kill multiple tasks at the same time; each oom
    killed task must exit before another task may be killed. Thus, if one
    thread is holding mm->mmap_sem and cannot allocate memory, all threads
    sharing the same ->mm are blocked from exiting as well. In the oom kill
    case, that means the thread holding mm->mmap_sem will never free
    additional memory since it cannot get access to memory reserves and the
    thread that depends on it with access to memory reserves cannot exit
    because it cannot acquire the semaphore. Thus, the page allocators
    livelocks.

    When the oom killer is called and current happens to have a pending
    SIGKILL, this patch automatically gives it access to memory reserves and
    returns. Upon returning to the page allocator, its allocation will
    hopefully succeed so it can quickly exit and free its memory. If not, the
    page allocator will fail the allocation if it is not __GFP_NOFAIL.

    Reviewed-by: KAMEZAWA Hiroyuki
    Signed-off-by: David Rientjes
    Cc: KOSAKI Motohiro
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • When find_lock_task_mm() returns a thread other than p in dump_tasks(),
    its name should be displayed instead. This is the thread that will be
    targeted by the oom killer, not its mm-less parent.

    This also allows us to safely dereference task->comm without needing
    get_task_comm().

    While we're here, remove the cast on task_cpu(task) as Andrew suggested.

    Signed-off-by: David Rientjes
    Cc: KOSAKI Motohiro
    Cc: Balbir Singh
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • The comments in dump_tasks() should be updated to be more clear about why
    tasks are filtered and how they are filtered by its argument.

    An unnecessary comment concerning a check for is_global_init() is removed
    since it isn't of importance.

    Suggested-by: Andrew Morton
    Signed-off-by: David Rientjes
    Acked-by: KOSAKI Motohiro
    Cc: Balbir Singh
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     
  • dump_task() should use find_lock_task_mm() too. It is necessary for
    protecting task-exiting race.

    dump_tasks() currently filters any task that does not have an attached
    ->mm since it incorrectly assumes that it must either be in the process of
    exiting and has detached its memory or that it's a kernel thread;
    multithreaded tasks may actually have subthreads that have a valid ->mm
    pointer and thus those threads should actually be displayed. This change
    finds those threads, if they exist, and emit their information along with
    the rest of the candidate tasks for kill.

    Signed-off-by: KOSAKI Motohiro
    Signed-off-by: David Rientjes
    Cc: Balbir Singh
    Cc: Oleg Nesterov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KOSAKI Motohiro
     
  • Almost all ->mm == NULL checks in oom_kill.c are wrong.

    The current code assumes that the task without ->mm has already released
    its memory and ignores the process. However this is not necessarily true
    when this process is multithreaded, other live sub-threads can use this
    ->mm.

    - Remove the "if (!p->mm)" check in select_bad_process(), it is
    just wrong.

    - Add the new helper, find_lock_task_mm(), which finds the live
    thread which uses the memory and takes task_lock() to pin ->mm

    - change oom_badness() to use this helper instead of just checking
    ->mm != NULL.

    - As David pointed out, select_bad_process() must never choose the
    task without ->mm, but no matter what oom_badness() returns the
    task can be chosen if nothing else has been found yet.

    Change oom_badness() to return int, change it to return -1 if
    find_lock_task_mm() fails, and change select_bad_process() to
    check points >= 0.

    Note! This patch is not enough, we need more changes.

    - oom_badness() was fixed, but oom_kill_task() still ignores
    the task without ->mm

    - oom_forkbomb_penalty() should use find_lock_task_mm() too,
    and it also needs other changes to actually find the first
    first-descendant children

    This will be addressed later.

    [kosaki.motohiro@jp.fujitsu.com: use in badness(), __oom_kill_task()]
    Signed-off-by: Oleg Nesterov
    Signed-off-by: David Rientjes
    Signed-off-by: KOSAKI Motohiro
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Oleg Nesterov
     
  • select_bad_process() checks PF_EXITING to detect the task which is going
    to release its memory, but the logic is very wrong.

    - a single process P with the dead group leader disables
    select_bad_process() completely, it will always return
    ERR_PTR() while P can live forever

    - if the PF_EXITING task has already released its ->mm
    it doesn't make sense to expect it is goiing to free
    more memory (except task_struct/etc)

    Change the code to ignore the PF_EXITING tasks without ->mm.

    Signed-off-by: Oleg Nesterov
    Signed-off-by: David Rientjes
    Cc: Balbir Singh
    Acked-by: KOSAKI Motohiro
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Oleg Nesterov
     
  • select_bad_process() thinks a kernel thread can't have ->mm != NULL, this
    is not true due to use_mm().

    Change the code to check PF_KTHREAD.

    Reviewed-by: KAMEZAWA Hiroyuki
    Signed-off-by: Oleg Nesterov
    Signed-off-by: David Rientjes
    Acked-by: KOSAKI Motohiro
    Cc: Balbir Singh
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Oleg Nesterov
     

28 May, 2010

1 commit

  • It's pointless to try to kill current if select_bad_process() did not find
    an eligible task to kill in mem_cgroup_out_of_memory() since it's
    guaranteed that current is a member of the memcg that is oom and it is, by
    definition, unkillable.

    Signed-off-by: David Rientjes
    Acked-by: KAMEZAWA Hiroyuki
    Cc: Balbir Singh
    Cc: Li Zefan
    Cc: Daisuke Nishimura
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Rientjes
     

30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

13 Mar, 2010

2 commits

  • In current page-fault code,

    handle_mm_fault()
    -> ...
    -> mem_cgroup_charge()
    -> map page or handle error.
    -> check return code.

    If page fault's return code is VM_FAULT_OOM, page_fault_out_of_memory() is
    called. But if it's caused by memcg, OOM should have been already
    invoked.

    Then, I added a patch: a636b327f731143ccc544b966cfd8de6cb6d72c6. That
    patch records last_oom_jiffies for memcg's sub-hierarchy and prevents
    page_fault_out_of_memory from being invoked in near future.

    But Nishimura-san reported that check by jiffies is not enough when the
    system is terribly heavy.

    This patch changes memcg's oom logic as.
    * If memcg causes OOM-kill, continue to retry.
    * remove jiffies check which is used now.
    * add memcg-oom-lock which works like perzone oom lock.
    * If current is killed(as a process), bypass charge.

    Something more sophisticated can be added but this pactch does
    fundamental things.
    TODO:
    - add oom notifier
    - add permemcg disable-oom-kill flag and freezer at oom.
    - more chances for wake up oom waiter (when changing memory limit etc..)

    Reviewed-by: Daisuke Nishimura
    Tested-by: Daisuke Nishimura
    Signed-off-by: KAMEZAWA Hiroyuki
    Cc: Balbir Singh
    Cc: David Rientjes
    Signed-off-by: Daisuke Nishimura
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KAMEZAWA Hiroyuki
     
  • Presently, if panic_on_oom=2, the whole system panics even if the oom
    happend in some special situation (as cpuset, mempolicy....). Then,
    panic_on_oom=2 means painc_on_oom_always.

    Now, memcg doesn't check panic_on_oom flag. This patch adds a check.

    BTW, how it's useful ?

    kdump+panic_on_oom=2 is the last tool to investigate what happens in
    oom-ed system. When a task is killed, the sysytem recovers and there will
    be few hint to know what happnes. In mission critical system, oom should
    never happen. Then, panic_on_oom=2+kdump is useful to avoid next OOM by
    knowing precise information via snapshot.

    TODO:
    - For memcg, it's for isolate system's memory usage, oom-notiifer and
    freeze_at_oom (or rest_at_oom) should be implemented. Then, management
    daemon can do similar jobs (as kdump) or taking snapshot per cgroup.

    Signed-off-by: KAMEZAWA Hiroyuki
    Cc: Balbir Singh
    Cc: David Rientjes
    Cc: Nick Piggin
    Reviewed-by: Daisuke Nishimura
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KAMEZAWA Hiroyuki
     

07 Mar, 2010

1 commit

  • Presently, per-mm statistics counter is defined by macro in sched.h

    This patch modifies it to
    - defined in mm.h as inlinf functions
    - use array instead of macro's name creation.

    This patch is for reducing patch size in future patch to modify
    implementation of per-mm counter.

    Signed-off-by: KAMEZAWA Hiroyuki
    Reviewed-by: Minchan Kim
    Cc: Christoph Lameter
    Cc: Lee Schermerhorn
    Cc: David Rientjes
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KAMEZAWA Hiroyuki
     

23 Feb, 2010

1 commit

  • Presently the oom-killer is memcg aware and it finds the worst process
    from processes under memcg(s) in oom. Then, it kills victim's child
    first.

    It may kill a child in another cgroup and may not be any help for
    recovery. And it will break the assumption users have.

    This patch fixes it.

    Signed-off-by: KAMEZAWA Hiroyuki
    Reviewed-by: Minchan Kim
    Cc: Balbir Singh
    Reviewed-by: Daisuke Nishimura
    Acked-by: David Rientjes
    Cc:
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    KAMEZAWA Hiroyuki