From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f43.google.com (mail-wm0-f43.google.com [74.125.82.43]) by kanga.kvack.org (Postfix) with ESMTP id 385376B0009 for ; Thu, 28 Jan 2016 15:11:29 -0500 (EST) Received: by mail-wm0-f43.google.com with SMTP id p63so40557396wmp.1 for ; Thu, 28 Jan 2016 12:11:29 -0800 (PST) Received: from mx2.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id g15si6272322wmd.12.2016.01.28.12.11.26 for (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 28 Jan 2016 12:11:28 -0800 (PST) Date: Thu, 28 Jan 2016 21:11:23 +0100 From: Michal Hocko Subject: Re: why do we do ALLOC_WMARK_HIGH before going out_of_memory Message-ID: <20160128201123.GB621@dhcp22.suse.cz> References: <20160128163802.GA15953@dhcp22.suse.cz> <20160128190204.GJ12228@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160128190204.GJ12228@redhat.com> Sender: owner-linux-mm@kvack.org List-ID: To: Andrea Arcangeli Cc: linux-mm@kvack.org, Mel Gorman , Johannes Weiner , David Rientjes , Andrew Morton On Thu 28-01-16 20:02:04, Andrea Arcangeli wrote: > Hello Michal, > > On Thu, Jan 28, 2016 at 05:38:03PM +0100, Michal Hocko wrote: > > Hi, > > __alloc_pages_may_oom just after it manages to get oom_lock we try > > to allocate once more with ALLOC_WMARK_HIGH target. I was always > > wondering why are we will to actually kill something even though > > we are above min wmark. This doesn't make much sense to me. I understand > > that this is racy because __alloc_pages_may_oom is called after we have > > failed to fulfill the WMARK_MIN target but this means WMARK_HIGH > > is highly unlikely as well. So either we should use ALLOC_WMARK_MIN > > or get rid of this altogether. > > > > The code has been added before git era by > > https://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11-rc2/2.6.11-rc2-mm2/broken-out/mm-fix-several-oom-killer-bugs.patch > > I assume you refer to this: > > + /* > + * Go through the zonelist yet one more time, keep > + * very high watermark here, this is only to catch > + * a parallel oom killing, we must fail if we're still > + * under heavy pressure. > + */ > + for (i = 0; (z = zones[i]) != NULL; i++) { > + if (!zone_watermark_ok(z, order, z->pages_high, > ^^^^^^^^^^^^^ yes > > and it doesn't explain this particular decision. It seems to me that > > Not explained explicitly in the commit header but see the above > comment added just before the z->pages_high, it at least tries to > explain it.. > > Although the implementation changed and now it's ALLOC_WMARK_HIGH > instead of z->pages_high, the old comment is still in the current > upstream: > > /* > * Go through the zonelist yet one more time, keep very high watermark > * here, this is only to catch a parallel oom killing, we must fail if > * we're still under heavy pressure. > */ Yes I have read the comment but it doesn't make any sense to me, to be honest. > > what ever was the reason back then it doesn't hold anymore. > > > > What do you think? > > Elaborating the comment: the reason for the high wmark is to reduce > the likelihood of livelocks and be sure to invoke the OOM killer, if > we're still under pressure and reclaim just failed. The high wmark is > used to be sure the failure of reclaim isn't going to be ignored. If > using the min wmark like you propose there's risk of livelock or > anyway of delayed OOM killer invocation. By livelock you mean trashing when last few pages are recycled very quickly and the OOM killer should be invoked instead? > The reason for doing one last wmark check (regardless of the wmark > used) before invoking the oom killer, was just to be sure another OOM > killer invocation hasn't already freed a ton of memory while we were > stuck in reclaim. A lot of free memory generated by the OOM killer, > won't make a parallel reclaim more likely to succeed, it just creates > free memory, but reclaim only succeeds when it finds "freeable" memory > and it makes progress in converting it to free memory. So for the > purpose of this last check, the high wmark would work fine as lots of > free memory would have been generated in such case. OK, I see. It is true that we try to allocate only if the direct reclaim made some progress which is not aware of the oom killer reclaimed memory. > > It's not immediately apparent if there is a new OOM killer upstream > logic that would prevent the risk of a second OOM killer invocation > despite another OOM killing already happened while we were stuck in > reclaim. In absence of that, the high wmark check would be still > needed. Well, my oom detection rework [1] strives to make the OOM detection more robust and the retry logic performs the watermark check. So I think the last attempt is no longer needed after that patch. I will then remove it. Thanks for the clarification --- [1] http://lkml.kernel.org/r/1450203586-10959-1-git-send-email-mhocko@kernel.org -- 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: email@kvack.org