On Wed 11-01-12 19:24:20, David Rientjes wrote:
> oom_kill_task() returns non-zero iff the chosen process does not have any
> threads with an attached ->mm.
>
> In such a case, it's better to just return to the page allocator and
> retry the allocation because memory could have been freed in the interim
> and the oom condition may no longer exist. It's unnecessary to loop in
> the oom killer and find another thread to kill.
>
> This allows both oom_kill_task() and oom_kill_process() to be converted
> to void functions. If the oom condition persists, the oom killer will be
> recalled.
>
> Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
Makes sense and the code is easier to read
Reviewed-by: Michal Hocko <mhocko@xxxxxxx>
Thanks
> ---
> mm/oom_kill.c | 56 ++++++++++++++++++++------------------------------------
> 1 files changed, 20 insertions(+), 36 deletions(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -434,14 +434,14 @@ static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order,
> }
>
> #define K(x) ((x) << (PAGE_SHIFT-10))
> -static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
> +static void oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
> {
> struct task_struct *q;
> struct mm_struct *mm;
>
> p = find_lock_task_mm(p);
> if (!p)
> - return 1;
> + return;
>
> /* mm cannot be safely dereferenced after task_unlock(p) */
> mm = p->mm;
> @@ -477,15 +477,13 @@ static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
>
> set_tsk_thread_flag(p, TIF_MEMDIE);
> force_sig(SIGKILL, p);
> -
> - return 0;
> }
> #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)
> +static void 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)
> {
> struct task_struct *victim = p;
> struct task_struct *child;
> @@ -501,7 +499,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
> */
> if (p->flags & PF_EXITING) {
> set_tsk_thread_flag(p, TIF_MEMDIE);
> - return 0;
> + return;
> }
>
> task_lock(p);
> @@ -533,7 +531,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
> }
> } while_each_thread(p, t);
>
> - return oom_kill_task(victim, mem);
> + oom_kill_task(victim, mem);
> }
>
> /*
> @@ -580,15 +578,10 @@ void mem_cgroup_out_of_memory(struct mem_cgroup *mem, gfp_t gfp_mask)
> check_panic_on_oom(CONSTRAINT_MEMCG, gfp_mask, 0, NULL);
> limit = mem_cgroup_get_limit(mem) >> PAGE_SHIFT;
> read_lock(&tasklist_lock);
> -retry:
> 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, limit, mem, NULL,
> - "Memory cgroup out of memory"))
> - goto retry;
> -out:
> + if (p && PTR_ERR(p) != -1UL)
> + oom_kill_process(p, gfp_mask, 0, points, limit, mem, NULL,
> + "Memory cgroup out of memory");
> read_unlock(&tasklist_lock);
> }
> #endif
> @@ -745,33 +738,24 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
> if (sysctl_oom_kill_allocating_task &&
> !oom_unkillable_task(current, NULL, nodemask) &&
> current->mm) {
> - /*
> - * oom_kill_process() needs tasklist_lock held. If it returns
> - * 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,
> - "Out of memory (oom_kill_allocating_task)"))
> - goto out;
> + oom_kill_process(current, gfp_mask, order, 0, totalpages, NULL,
> + nodemask,
> + "Out of memory (oom_kill_allocating_task)");
> + goto out;
> }
>
> -retry:
> p = select_bad_process(&points, totalpages, NULL, mpol_mask);
> - if (PTR_ERR(p) == -1UL)
> - goto out;
> -
> /* Found nothing?!?! Either we hang forever, or we panic. */
> if (!p) {
> dump_header(NULL, gfp_mask, order, NULL, mpol_mask);
> read_unlock(&tasklist_lock);
> panic("Out of memory and no killable processes...\n");
> }
> -
> - if (oom_kill_process(p, gfp_mask, order, points, totalpages, NULL,
> - nodemask, "Out of memory"))
> - goto retry;
> - killed = 1;
> + if (PTR_ERR(p) != -1UL) {
> + oom_kill_process(p, gfp_mask, order, points, totalpages, NULL,
> + nodemask, "Out of memory");
> + killed = 1;
> + }
> out:
> read_unlock(&tasklist_lock);
>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>