From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> To: mhocko@kernel.org Cc: rientjes@google.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [patch] mm, oom: prevent additional oom kills before memory is freed Date: Thu, 15 Jun 2017 22:01:33 +0900 [thread overview] Message-ID: <201706152201.CAB48456.FtHOJMFOVLSFQO@I-love.SAKURA.ne.jp> (raw) In-Reply-To: <20170615121315.GK1486@dhcp22.suse.cz> Michal Hocko wrote: > On Thu 15-06-17 14:03:35, Michal Hocko wrote: > > On Thu 15-06-17 20:32:39, Tetsuo Handa wrote: > > > @@ -556,25 +553,21 @@ static void oom_reap_task(struct task_struct *tsk) > > > struct mm_struct *mm = tsk->signal->oom_mm; > > > > > > /* Retry the down_read_trylock(mmap_sem) a few times */ > > > - while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task_mm(tsk, mm)) > > > + while (__oom_reap_task_mm(tsk, mm), !test_bit(MMF_OOM_SKIP, &mm->flags) > > > + && attempts++ < MAX_OOM_REAP_RETRIES) > > > schedule_timeout_idle(HZ/10); > > > > > > - if (attempts <= MAX_OOM_REAP_RETRIES) > > > - goto done; > > > - > > > - > > > - pr_info("oom_reaper: unable to reap pid:%d (%s)\n", > > > - task_pid_nr(tsk), tsk->comm); > > > - debug_show_all_locks(); > > > - > > > -done: > > > - tsk->oom_reaper_list = NULL; > > > - > > > /* > > > * Hide this mm from OOM killer because it has been either reaped or > > > * somebody can't call up_write(mmap_sem). > > > */ > > > - set_bit(MMF_OOM_SKIP, &mm->flags); > > > + if (!test_and_set_bit(MMF_OOM_SKIP, &mm->flags)) { > > > + pr_info("oom_reaper: unable to reap pid:%d (%s)\n", > > > + task_pid_nr(tsk), tsk->comm); > > > + debug_show_all_locks(); > > > + } > > > + > > > > How does this _solve_ anything? Why would you even retry when you > > _know_ that the reference count dropped to zero. It will never > > increment. So the above is basically just schedule_timeout_idle(HZ/10) * > > MAX_OOM_REAP_RETRIES before we set MMF_OOM_SKIP. If the OOM reaper knows that mm->users == 0, it gives __mmput() some time to "complete exit_mmap() etc. and set MMF_OOM_SKIP". If __mmput() released some memory, subsequent OOM killer invocation is automatically avoided. If __mmput() did not release some memory, let the OOM killer invoke again. > > Just to make myself more clear. The above assumes that the victim hasn't > passed exit_mmap and MMF_OOM_SKIP in __mmput. Which is the case we want to > address here. David is trying to avoid setting MMF_OOM_SKIP when the OOM reaper found that mm->users == 0. But we must not wait forever because __mmput() might fail to release some memory immediately. If __mmput() did not release some memory within schedule_timeout_idle(HZ/10) * MAX_OOM_REAP_RETRIES sleep, let the OOM killer invoke again. So, this is the case we want to address here, isn't it?
WARNING: multiple messages have this Message-ID (diff)
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> To: mhocko@kernel.org Cc: rientjes@google.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [patch] mm, oom: prevent additional oom kills before memory is freed Date: Thu, 15 Jun 2017 22:01:33 +0900 [thread overview] Message-ID: <201706152201.CAB48456.FtHOJMFOVLSFQO@I-love.SAKURA.ne.jp> (raw) In-Reply-To: <20170615121315.GK1486@dhcp22.suse.cz> Michal Hocko wrote: > On Thu 15-06-17 14:03:35, Michal Hocko wrote: > > On Thu 15-06-17 20:32:39, Tetsuo Handa wrote: > > > @@ -556,25 +553,21 @@ static void oom_reap_task(struct task_struct *tsk) > > > struct mm_struct *mm = tsk->signal->oom_mm; > > > > > > /* Retry the down_read_trylock(mmap_sem) a few times */ > > > - while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task_mm(tsk, mm)) > > > + while (__oom_reap_task_mm(tsk, mm), !test_bit(MMF_OOM_SKIP, &mm->flags) > > > + && attempts++ < MAX_OOM_REAP_RETRIES) > > > schedule_timeout_idle(HZ/10); > > > > > > - if (attempts <= MAX_OOM_REAP_RETRIES) > > > - goto done; > > > - > > > - > > > - pr_info("oom_reaper: unable to reap pid:%d (%s)\n", > > > - task_pid_nr(tsk), tsk->comm); > > > - debug_show_all_locks(); > > > - > > > -done: > > > - tsk->oom_reaper_list = NULL; > > > - > > > /* > > > * Hide this mm from OOM killer because it has been either reaped or > > > * somebody can't call up_write(mmap_sem). > > > */ > > > - set_bit(MMF_OOM_SKIP, &mm->flags); > > > + if (!test_and_set_bit(MMF_OOM_SKIP, &mm->flags)) { > > > + pr_info("oom_reaper: unable to reap pid:%d (%s)\n", > > > + task_pid_nr(tsk), tsk->comm); > > > + debug_show_all_locks(); > > > + } > > > + > > > > How does this _solve_ anything? Why would you even retry when you > > _know_ that the reference count dropped to zero. It will never > > increment. So the above is basically just schedule_timeout_idle(HZ/10) * > > MAX_OOM_REAP_RETRIES before we set MMF_OOM_SKIP. If the OOM reaper knows that mm->users == 0, it gives __mmput() some time to "complete exit_mmap() etc. and set MMF_OOM_SKIP". If __mmput() released some memory, subsequent OOM killer invocation is automatically avoided. If __mmput() did not release some memory, let the OOM killer invoke again. > > Just to make myself more clear. The above assumes that the victim hasn't > passed exit_mmap and MMF_OOM_SKIP in __mmput. Which is the case we want to > address here. David is trying to avoid setting MMF_OOM_SKIP when the OOM reaper found that mm->users == 0. But we must not wait forever because __mmput() might fail to release some memory immediately. If __mmput() did not release some memory within schedule_timeout_idle(HZ/10) * MAX_OOM_REAP_RETRIES sleep, let the OOM killer invoke again. So, this is the case we want to address here, isn't it? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-06-15 13:01 UTC|newest] Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-14 23:43 [patch] mm, oom: prevent additional oom kills before memory is freed David Rientjes 2017-06-14 23:43 ` David Rientjes 2017-06-15 10:39 ` Michal Hocko 2017-06-15 10:39 ` Michal Hocko 2017-06-15 10:53 ` Tetsuo Handa 2017-06-15 10:53 ` Tetsuo Handa 2017-06-15 11:01 ` Michal Hocko 2017-06-15 11:01 ` Michal Hocko 2017-06-15 11:32 ` Tetsuo Handa 2017-06-15 11:32 ` Tetsuo Handa 2017-06-15 12:03 ` Michal Hocko 2017-06-15 12:03 ` Michal Hocko 2017-06-15 12:13 ` Michal Hocko 2017-06-15 12:13 ` Michal Hocko 2017-06-15 13:01 ` Tetsuo Handa [this message] 2017-06-15 13:01 ` Tetsuo Handa 2017-06-15 13:22 ` Michal Hocko 2017-06-15 13:22 ` Michal Hocko 2017-06-15 21:43 ` Tetsuo Handa 2017-06-15 21:43 ` Tetsuo Handa 2017-06-15 21:37 ` David Rientjes 2017-06-15 21:37 ` David Rientjes 2017-06-15 12:20 ` Michal Hocko 2017-06-15 12:20 ` Michal Hocko 2017-06-15 21:26 ` David Rientjes 2017-06-15 21:26 ` David Rientjes 2017-06-15 21:41 ` Michal Hocko 2017-06-15 21:41 ` Michal Hocko 2017-06-15 22:03 ` David Rientjes 2017-06-15 22:03 ` David Rientjes 2017-06-15 22:12 ` Michal Hocko 2017-06-15 22:12 ` Michal Hocko 2017-06-15 22:42 ` David Rientjes 2017-06-15 22:42 ` David Rientjes 2017-06-16 8:06 ` Michal Hocko 2017-06-16 8:06 ` Michal Hocko 2017-06-16 0:54 ` Tetsuo Handa 2017-06-16 0:54 ` Tetsuo Handa 2017-06-16 4:00 ` Tetsuo Handa 2017-06-16 4:00 ` Tetsuo Handa 2017-06-16 8:39 ` Michal Hocko 2017-06-16 8:39 ` Michal Hocko 2017-06-16 10:27 ` Tetsuo Handa 2017-06-16 10:27 ` Tetsuo Handa 2017-06-16 11:02 ` Michal Hocko 2017-06-16 11:02 ` Michal Hocko 2017-06-16 14:26 ` Re: [patch] mm, oom: prevent additional oom kills before memoryis freed Tetsuo Handa 2017-06-16 14:26 ` Tetsuo Handa 2017-06-16 14:42 ` Michal Hocko 2017-06-16 14:42 ` Michal Hocko 2017-06-17 13:30 ` Re: [patch] mm, oom: prevent additional oom kills before memory is freed Tetsuo Handa 2017-06-17 13:30 ` Tetsuo Handa 2017-06-23 12:38 ` Michal Hocko 2017-06-23 12:38 ` Michal Hocko 2017-06-16 12:22 ` Tetsuo Handa 2017-06-16 12:22 ` Tetsuo Handa 2017-06-16 14:12 ` Michal Hocko 2017-06-16 14:12 ` Michal Hocko 2017-06-17 5:17 ` [PATCH] mm,oom_kill: Close race window of needlessly selecting new victims Tetsuo Handa 2017-06-17 5:17 ` Tetsuo Handa 2017-06-20 22:12 ` David Rientjes 2017-06-20 22:12 ` David Rientjes 2017-06-21 2:17 ` Tetsuo Handa 2017-06-21 20:31 ` David Rientjes 2017-06-21 20:31 ` David Rientjes 2017-06-22 0:53 ` Tetsuo Handa 2017-06-23 12:45 ` Michal Hocko 2017-06-23 12:45 ` Michal Hocko 2017-06-21 13:18 ` Michal Hocko 2017-06-21 13:18 ` Michal Hocko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=201706152201.CAB48456.FtHOJMFOVLSFQO@I-love.SAKURA.ne.jp \ --to=penguin-kernel@i-love.sakura.ne.jp \ --cc=akpm@linux-foundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@kernel.org \ --cc=rientjes@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.