From: peter enderborg <peter.enderborg@sony.com> To: Michal Hocko <mhocko@kernel.org> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, <linux-mm@kvack.org>, <rientjes@google.com>, <akpm@linux-foundation.org>, <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] mm,oom: Bring OOM notifier callbacks to outside of OOM killer. Date: Mon, 25 Jun 2018 16:04:04 +0200 [thread overview] Message-ID: <9a14d554-6470-e0d6-19cc-1ecec17a47c7@sony.com> (raw) In-Reply-To: <20180625130756.GK28965@dhcp22.suse.cz> On 06/25/2018 03:07 PM, Michal Hocko wrote: > On Mon 25-06-18 15:03:40, peter enderborg wrote: >> On 06/20/2018 01:55 PM, Michal Hocko wrote: >>> On Wed 20-06-18 20:20:38, Tetsuo Handa wrote: >>>> Sleeping with oom_lock held can cause AB-BA lockup bug because >>>> __alloc_pages_may_oom() does not wait for oom_lock. Since >>>> blocking_notifier_call_chain() in out_of_memory() might sleep, sleeping >>>> with oom_lock held is currently an unavoidable problem. >>> Could you be more specific about the potential deadlock? Sleeping while >>> holding oom lock is certainly not nice but I do not see how that would >>> result in a deadlock assuming that the sleeping context doesn't sleep on >>> the memory allocation obviously. >> It is a mutex you are supposed to be able to sleep. It's even exported. > What do you mean? oom_lock is certainly not exported for general use. It > is not local to oom_killer.c just because it is needed in other _mm_ > code. > It is in the oom.h file include/linux/oom.h, if it that sensitive it should be in mm/ and a documented note about the special rules. It is only used in drivers/tty/sysrq.c and that be replaced by a help function in mm that do the oom stuff. >>>> As a preparation for not to sleep with oom_lock held, this patch brings >>>> OOM notifier callbacks to outside of OOM killer, with two small behavior >>>> changes explained below. >>> Can we just eliminate this ugliness and remove it altogether? We do not >>> have that many notifiers. Is there anything fundamental that would >>> prevent us from moving them to shrinkers instead? >> @Hocko Do you remember the lowmemorykiller from android? Some things >> might not be the right thing for shrinkers. > Just that lmk did it wrong doesn't mean others have to follow. > If all you have is a hammer, everything looks like a nail. (I don’t argument that it was right) But if you don’t have a way to interact with the memory system we will get attempts like lmk. Oom notifiers and vmpressure is for this task better than shrinkers.
WARNING: multiple messages have this Message-ID (diff)
From: peter enderborg <peter.enderborg@sony.com> To: Michal Hocko <mhocko@kernel.org> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, linux-mm@kvack.org, rientjes@google.com, akpm@linux-foundation.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm,oom: Bring OOM notifier callbacks to outside of OOM killer. Date: Mon, 25 Jun 2018 16:04:04 +0200 [thread overview] Message-ID: <9a14d554-6470-e0d6-19cc-1ecec17a47c7@sony.com> (raw) In-Reply-To: <20180625130756.GK28965@dhcp22.suse.cz> On 06/25/2018 03:07 PM, Michal Hocko wrote: > On Mon 25-06-18 15:03:40, peter enderborg wrote: >> On 06/20/2018 01:55 PM, Michal Hocko wrote: >>> On Wed 20-06-18 20:20:38, Tetsuo Handa wrote: >>>> Sleeping with oom_lock held can cause AB-BA lockup bug because >>>> __alloc_pages_may_oom() does not wait for oom_lock. Since >>>> blocking_notifier_call_chain() in out_of_memory() might sleep, sleeping >>>> with oom_lock held is currently an unavoidable problem. >>> Could you be more specific about the potential deadlock? Sleeping while >>> holding oom lock is certainly not nice but I do not see how that would >>> result in a deadlock assuming that the sleeping context doesn't sleep on >>> the memory allocation obviously. >> It is a mutex you are supposed to be able to sleep. It's even exported. > What do you mean? oom_lock is certainly not exported for general use. It > is not local to oom_killer.c just because it is needed in other _mm_ > code. > It is in the oom.h file include/linux/oom.h, if it that sensitive it should be in mm/ and a documented note about the special rules. It is only used in drivers/tty/sysrq.c and that be replaced by a help function in mm that do the oom stuff. >>>> As a preparation for not to sleep with oom_lock held, this patch brings >>>> OOM notifier callbacks to outside of OOM killer, with two small behavior >>>> changes explained below. >>> Can we just eliminate this ugliness and remove it altogether? We do not >>> have that many notifiers. Is there anything fundamental that would >>> prevent us from moving them to shrinkers instead? >> @Hocko Do you remember the lowmemorykiller from android? Some things >> might not be the right thing for shrinkers. > Just that lmk did it wrong doesn't mean others have to follow. > If all you have is a hammer, everything looks like a nail. (I don’t argument that it was right) But if you don’t have a way to interact with the memory system we will get attempts like lmk. Oom notifiers and vmpressure is for this task better than shrinkers.
next prev parent reply other threads:[~2018-06-25 14:04 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-06-20 11:20 [PATCH] mm,oom: Bring OOM notifier callbacks to outside of OOM killer Tetsuo Handa 2018-06-20 11:55 ` Michal Hocko 2018-06-20 12:21 ` Tetsuo Handa 2018-06-20 13:07 ` Michal Hocko 2018-06-25 13:03 ` peter enderborg 2018-06-25 13:03 ` peter enderborg 2018-06-25 13:07 ` Michal Hocko 2018-06-25 13:07 ` Michal Hocko 2018-06-25 14:02 ` peter enderborg 2018-06-25 14:02 ` peter enderborg 2018-06-25 14:04 ` peter enderborg [this message] 2018-06-25 14:04 ` peter enderborg 2018-06-25 14:12 ` Michal Hocko 2018-06-25 14:12 ` Michal Hocko 2018-06-20 22:36 ` David Rientjes 2018-06-21 7:31 ` Michal Hocko 2018-06-21 11:27 ` Tetsuo Handa 2018-06-21 12:05 ` Michal Hocko 2018-06-26 17:03 ` Paul E. McKenney 2018-06-26 20:10 ` Tetsuo Handa 2018-06-26 23:50 ` Paul E. McKenney 2018-06-27 10:52 ` Tetsuo Handa 2018-06-27 14:28 ` Paul E. McKenney 2018-06-27 7:22 ` Michal Hocko 2018-06-27 14:31 ` Paul E. McKenney 2018-06-28 11:39 ` Michal Hocko 2018-06-28 21:31 ` Paul E. McKenney 2018-06-29 9:04 ` Michal Hocko 2018-06-29 12:52 ` Paul E. McKenney 2018-06-29 13:26 ` Michal Hocko 2018-06-30 17:05 ` Paul E. McKenney 2018-07-02 12:00 ` Michal Hocko 2018-07-02 21:37 ` Paul E. McKenney 2018-07-03 7:24 ` Michal Hocko 2018-07-03 16:01 ` Paul E. McKenney 2018-07-06 5:39 ` Michal Hocko 2018-07-06 12:22 ` Paul E. McKenney 2018-06-29 14:35 ` Tetsuo Handa 2018-06-30 17:19 ` Paul E. McKenney
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=9a14d554-6470-e0d6-19cc-1ecec17a47c7@sony.com \ --to=peter.enderborg@sony.com \ --cc=akpm@linux-foundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@kernel.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.