From: Michal Hocko <mhocko@kernel.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: Mel Gorman <mgorman@suse.de>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, David Rientjes <rientjes@google.com>, Linus Torvalds <torvalds@linux-foundation.org>, Oleg Nesterov <oleg@redhat.com>, Hugh Dickins <hughd@google.com>, Andrea Argangeli <andrea@kernel.org>, Rik van Riel <riel@redhat.com>, linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/2] mm, oom: introduce oom reaper Date: Thu, 17 Dec 2015 14:02:24 +0100 [thread overview] Message-ID: <20151217130223.GE18625@dhcp22.suse.cz> (raw) In-Reply-To: <20151216165035.38a4d9b84600d6348a3cf4bf@linux-foundation.org> On Wed 16-12-15 16:50:35, Andrew Morton wrote: > On Tue, 15 Dec 2015 19:36:15 +0100 Michal Hocko <mhocko@kernel.org> wrote: [...] > > +static void oom_reap_vmas(struct mm_struct *mm) > > +{ > > + int attempts = 0; > > + > > + while (attempts++ < 10 && !__oom_reap_vmas(mm)) > > + schedule_timeout(HZ/10); > > schedule_timeout() in state TASK_RUNNING doesn't do anything. Use > msleep() or msleep_interruptible(). I can't decide which is more > appropriate - it only affects the load average display. Ups. You are right. I will go with msleep_interruptible(100). > Which prompts the obvious question: as the no-operativeness of this > call wasn't noticed in testing, why do we have it there... Well, the idea was that an interfering mmap_sem operation which holds it for write might block us for a short time period - e.g. when not depending on an allocation or accessing the memory reserves helps to progress the allocation. If the holder of the semaphore is stuck then the retry is pointless. On the other hand the retry shouldn't be harmful. All in all this is just a heuristic and we do not depend on it. I guess we can drop it and nobody would actually notice. Let me know if you prefer that and I will respin the patch. > I guess it means that the __oom_reap_vmas() success rate is nice anud > high ;) I had a debugging trace_printks around this and there were no reties during my testing so I was probably lucky to not trigger the mmap_sem contention. --- diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 48025a21f8c4..f53f87cfd899 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -469,7 +469,7 @@ static void oom_reap_vmas(struct mm_struct *mm) int attempts = 0; while (attempts++ < 10 && !__oom_reap_vmas(mm)) - schedule_timeout(HZ/10); + msleep_interruptible(100); /* Drop a reference taken by wake_oom_reaper */ mmdrop(mm); -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: Mel Gorman <mgorman@suse.de>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, David Rientjes <rientjes@google.com>, Linus Torvalds <torvalds@linux-foundation.org>, Oleg Nesterov <oleg@redhat.com>, Hugh Dickins <hughd@google.com>, Andrea Argangeli <andrea@kernel.org>, Rik van Riel <riel@redhat.com>, linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/2] mm, oom: introduce oom reaper Date: Thu, 17 Dec 2015 14:02:24 +0100 [thread overview] Message-ID: <20151217130223.GE18625@dhcp22.suse.cz> (raw) In-Reply-To: <20151216165035.38a4d9b84600d6348a3cf4bf@linux-foundation.org> On Wed 16-12-15 16:50:35, Andrew Morton wrote: > On Tue, 15 Dec 2015 19:36:15 +0100 Michal Hocko <mhocko@kernel.org> wrote: [...] > > +static void oom_reap_vmas(struct mm_struct *mm) > > +{ > > + int attempts = 0; > > + > > + while (attempts++ < 10 && !__oom_reap_vmas(mm)) > > + schedule_timeout(HZ/10); > > schedule_timeout() in state TASK_RUNNING doesn't do anything. Use > msleep() or msleep_interruptible(). I can't decide which is more > appropriate - it only affects the load average display. Ups. You are right. I will go with msleep_interruptible(100). > Which prompts the obvious question: as the no-operativeness of this > call wasn't noticed in testing, why do we have it there... Well, the idea was that an interfering mmap_sem operation which holds it for write might block us for a short time period - e.g. when not depending on an allocation or accessing the memory reserves helps to progress the allocation. If the holder of the semaphore is stuck then the retry is pointless. On the other hand the retry shouldn't be harmful. All in all this is just a heuristic and we do not depend on it. I guess we can drop it and nobody would actually notice. Let me know if you prefer that and I will respin the patch. > I guess it means that the __oom_reap_vmas() success rate is nice anud > high ;) I had a debugging trace_printks around this and there were no reties during my testing so I was probably lucky to not trigger the mmap_sem contention. --- diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 48025a21f8c4..f53f87cfd899 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -469,7 +469,7 @@ static void oom_reap_vmas(struct mm_struct *mm) int attempts = 0; while (attempts++ < 10 && !__oom_reap_vmas(mm)) - schedule_timeout(HZ/10); + msleep_interruptible(100); /* Drop a reference taken by wake_oom_reaper */ mmdrop(mm); -- 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:[~2015-12-17 13:02 UTC|newest] Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-12-15 18:36 [PATCH 1/2] mm, oom: introduce oom reaper Michal Hocko 2015-12-15 18:36 ` Michal Hocko 2015-12-17 0:50 ` Andrew Morton 2015-12-17 0:50 ` Andrew Morton 2015-12-17 13:02 ` Michal Hocko [this message] 2015-12-17 13:02 ` Michal Hocko 2015-12-17 19:55 ` Linus Torvalds 2015-12-17 19:55 ` Linus Torvalds 2015-12-17 20:00 ` Andrew Morton 2015-12-17 20:00 ` Andrew Morton 2015-12-18 11:54 ` Michal Hocko 2015-12-18 11:54 ` Michal Hocko 2015-12-18 21:14 ` Andrew Morton 2015-12-18 21:14 ` Andrew Morton 2015-12-21 8:38 ` Michal Hocko 2015-12-21 8:38 ` Michal Hocko 2015-12-17 21:13 ` Andrew Morton 2015-12-17 21:13 ` Andrew Morton 2015-12-18 12:11 ` Michal Hocko 2015-12-18 12:11 ` Michal Hocko 2015-12-18 12:10 ` Tetsuo Handa 2015-12-18 12:10 ` Tetsuo Handa 2015-12-20 7:14 ` Tetsuo Handa 2015-12-20 7:14 ` Tetsuo Handa 2015-12-18 0:15 ` Andrew Morton 2015-12-18 0:15 ` Andrew Morton 2015-12-18 11:48 ` Michal Hocko 2015-12-18 11:48 ` Michal Hocko 2015-12-21 20:38 ` Paul Gortmaker 2015-12-21 20:38 ` Paul Gortmaker 2016-01-06 9:10 ` Michal Hocko 2016-01-06 9:10 ` Michal Hocko 2016-01-06 14:26 ` Paul Gortmaker 2016-01-06 14:26 ` Paul Gortmaker 2016-01-06 15:00 ` Michal Hocko 2016-01-06 15:00 ` Michal Hocko 2015-12-23 23:00 ` Ross Zwisler 2015-12-23 23:00 ` Ross Zwisler 2015-12-24 9:47 ` Michal Hocko 2015-12-24 9:47 ` Michal Hocko 2015-12-24 11:06 ` Tetsuo Handa 2015-12-24 11:06 ` Tetsuo Handa 2015-12-24 20:39 ` Ross Zwisler 2015-12-24 20:39 ` Ross Zwisler 2015-12-25 11:41 ` Michal Hocko 2015-12-25 11:41 ` Michal Hocko 2015-12-24 20:44 ` Ross Zwisler 2015-12-24 20:44 ` Ross Zwisler 2015-12-25 11:35 ` Michal Hocko 2015-12-25 11:35 ` Michal Hocko 2015-12-25 11:44 ` Michal Hocko 2015-12-25 11:44 ` Michal Hocko 2016-01-06 15:42 [PATCH 0/2 -mm] oom reaper v4 Michal Hocko 2016-01-06 15:42 ` [PATCH 1/2] mm, oom: introduce oom reaper Michal Hocko 2016-01-06 15:42 ` Michal Hocko 2016-01-07 11:23 ` Tetsuo Handa 2016-01-07 11:23 ` Tetsuo Handa 2016-01-07 12:30 ` Michal Hocko 2016-01-07 12:30 ` Michal Hocko 2016-01-11 22:54 ` Andrew Morton 2016-01-11 22:54 ` Andrew Morton 2016-01-12 8:16 ` Michal Hocko 2016-01-12 8:16 ` Michal Hocko 2016-01-28 1:28 ` David Rientjes 2016-01-28 1:28 ` David Rientjes 2016-01-28 21:42 ` Michal Hocko 2016-01-28 21:42 ` Michal Hocko 2016-02-02 3:02 ` David Rientjes 2016-02-02 3:02 ` David Rientjes 2016-02-02 8:57 ` Michal Hocko 2016-02-02 8:57 ` Michal Hocko 2016-02-02 11:48 ` Tetsuo Handa 2016-02-02 11:48 ` Tetsuo Handa 2016-02-02 22:55 ` David Rientjes 2016-02-02 22:55 ` David Rientjes 2016-02-02 22:51 ` David Rientjes 2016-02-02 22:51 ` David Rientjes 2016-02-03 10:31 ` Tetsuo Handa 2016-02-03 10:31 ` Tetsuo Handa
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=20151217130223.GE18625@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=andrea@kernel.org \ --cc=hughd@google.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@suse.de \ --cc=oleg@redhat.com \ --cc=penguin-kernel@I-love.SAKURA.ne.jp \ --cc=riel@redhat.com \ --cc=rientjes@google.com \ --cc=torvalds@linux-foundation.org \ /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.