Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

mm/oom_kill.c: clean up oom_reap_task_mm()

Andrew has noticed some inconsistencies in oom_reap_task_mm. Notably

- Undocumented return value.

- comment "failed to reap part..." is misleading - sounds like it's
referring to something which happened in the past, is in fact
referring to something which might happen in the future.

- fails to call trace_finish_task_reaping() in one case

- code duplication.

- Increases mmap_sem hold time a little by moving
trace_finish_task_reaping() inside the locked region. So sue me ;)

- Sharing the finish: path means that the trace event won't
distinguish between the two sources of finishing.

Add a short explanation for the return value and fix the rest by
reorganizing the function a bit to have unified function exit paths.

Link: http://lkml.kernel.org/r/20180724141747.GP28386@dhcp22.suse.cz
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: Andrew Morton <akpm@linux-foundation.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Michal Hocko and committed by
Linus Torvalds
431f42fd c3b78b11

+16 -8
+16 -8
mm/oom_kill.c
··· 534 534 return ret; 535 535 } 536 536 537 + /* 538 + * Reaps the address space of the give task. 539 + * 540 + * Returns true on success and false if none or part of the address space 541 + * has been reclaimed and the caller should retry later. 542 + */ 537 543 static bool oom_reap_task_mm(struct task_struct *tsk, struct mm_struct *mm) 538 544 { 545 + bool ret = true; 546 + 539 547 if (!down_read_trylock(&mm->mmap_sem)) { 540 548 trace_skip_task_reaping(tsk->pid); 541 549 return false; ··· 556 548 * down_write();up_write() cycle in exit_mmap(). 557 549 */ 558 550 if (test_bit(MMF_OOM_SKIP, &mm->flags)) { 559 - up_read(&mm->mmap_sem); 560 551 trace_skip_task_reaping(tsk->pid); 561 - return true; 552 + goto out_unlock; 562 553 } 563 554 564 555 trace_start_task_reaping(tsk->pid); 565 556 566 557 /* failed to reap part of the address space. Try again later */ 567 - if (!__oom_reap_task_mm(mm)) { 568 - up_read(&mm->mmap_sem); 569 - return false; 570 - } 558 + ret = __oom_reap_task_mm(mm); 559 + if (!ret) 560 + goto out_finish; 571 561 572 562 pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n", 573 563 task_pid_nr(tsk), tsk->comm, 574 564 K(get_mm_counter(mm, MM_ANONPAGES)), 575 565 K(get_mm_counter(mm, MM_FILEPAGES)), 576 566 K(get_mm_counter(mm, MM_SHMEMPAGES))); 567 + out_finish: 568 + trace_finish_task_reaping(tsk->pid); 569 + out_unlock: 577 570 up_read(&mm->mmap_sem); 578 571 579 - trace_finish_task_reaping(tsk->pid); 580 - return true; 572 + return ret; 581 573 } 582 574 583 575 #define MAX_OOM_REAP_RETRIES 10