From: Michal Hocko <mhocko@kernel.org> To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> 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 14:03:35 +0200 [thread overview] Message-ID: <20170615120335.GJ1486@dhcp22.suse.cz> (raw) In-Reply-To: <201706152032.BFE21313.MSHQOtLVFFJOOF@I-love.SAKURA.ne.jp> On Thu 15-06-17 20:32:39, Tetsuo Handa wrote: > Michal Hocko wrote: [...] > > An alternative would be to allow reaping and exit_mmap race. The unmap > > part should just work I guess. We just have to be careful to not race > > with free_pgtables and that shouldn't be too hard to implement (e.g. > > (ab)use mmap_sem for write there). I haven't thought that through > > completely though so I might miss something of course. > > I think below one is simpler. [...] > @@ -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. This might be enough for victim to finish the exit_mmap but it is more a hack^Wworkround than anything else. You could very well do the sleep without any obfuscation... -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> 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 14:03:35 +0200 [thread overview] Message-ID: <20170615120335.GJ1486@dhcp22.suse.cz> (raw) In-Reply-To: <201706152032.BFE21313.MSHQOtLVFFJOOF@I-love.SAKURA.ne.jp> On Thu 15-06-17 20:32:39, Tetsuo Handa wrote: > Michal Hocko wrote: [...] > > An alternative would be to allow reaping and exit_mmap race. The unmap > > part should just work I guess. We just have to be careful to not race > > with free_pgtables and that shouldn't be too hard to implement (e.g. > > (ab)use mmap_sem for write there). I haven't thought that through > > completely though so I might miss something of course. > > I think below one is simpler. [...] > @@ -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. This might be enough for victim to finish the exit_mmap but it is more a hack^Wworkround than anything else. You could very well do the sleep without any obfuscation... -- Michal Hocko SUSE Labs -- 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 12:03 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 [this message] 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 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=20170615120335.GJ1486@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=penguin-kernel@I-love.SAKURA.ne.jp \ --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.