From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030418AbbJ3JmI (ORCPT ); Fri, 30 Oct 2015 05:42:08 -0400 Received: from mgwym01.jp.fujitsu.com ([211.128.242.40]:63564 "EHLO mgwym01.jp.fujitsu.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965478AbbJ3JmG (ORCPT ); Fri, 30 Oct 2015 05:42:06 -0400 X-SecurityPolicyCheck: OK by SHieldMailChecker v2.3.2 X-SHieldMailCheckerPolicyVersion: FJ-ISEC-20150223 X-SHieldMailCheckerMailID: d21704514aa64d0791cb5e5b3b43c77b Subject: Re: [RFC 1/3] mm, oom: refactor oom detection To: Michal Hocko References: <1446131835-3263-1-git-send-email-mhocko@kernel.org> <1446131835-3263-2-git-send-email-mhocko@kernel.org> <5632FEEF.2050709@jp.fujitsu.com> <20151030082323.GB18429@dhcp22.suse.cz> Cc: linux-mm@kvack.org, Andrew Morton , Linus Torvalds , Mel Gorman , Johannes Weiner , Rik van Riel , David Rientjes , Tetsuo Handa , LKML From: Kamezawa Hiroyuki Message-ID: <56333B4A.4030602@jp.fujitsu.com> Date: Fri, 30 Oct 2015 18:41:30 +0900 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20151030082323.GB18429@dhcp22.suse.cz> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015/10/30 17:23, Michal Hocko wrote: > On Fri 30-10-15 14:23:59, KAMEZAWA Hiroyuki wrote: >> On 2015/10/30 0:17, mhocko@kernel.org wrote: > [...] >>> @@ -3135,13 +3145,56 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, >>> if (gfp_mask & __GFP_NORETRY) >>> goto noretry; >>> >>> - /* Keep reclaiming pages as long as there is reasonable progress */ >>> + /* >>> + * Do not retry high order allocations unless they are __GFP_REPEAT >>> + * and even then do not retry endlessly. >>> + */ >>> pages_reclaimed += did_some_progress; >>> - if ((did_some_progress && order <= PAGE_ALLOC_COSTLY_ORDER) || >>> - ((gfp_mask & __GFP_REPEAT) && pages_reclaimed < (1 << order))) { >>> - /* Wait for some write requests to complete then retry */ >>> - wait_iff_congested(ac->preferred_zone, BLK_RW_ASYNC, HZ/50); >>> - goto retry; >>> + if (order > PAGE_ALLOC_COSTLY_ORDER) { >>> + if (!(gfp_mask & __GFP_REPEAT) || pages_reclaimed >= (1<>> + goto noretry; >>> + >>> + if (did_some_progress) >>> + goto retry; >> >> why directly retry here ? > > Because I wanted to preserve the previous logic for GFP_REPEAT as much > as possible here and do an incremental change in the later patch. > I see. > [...] > >>> @@ -3150,8 +3203,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, >>> goto got_pg; >>> >>> /* Retry as long as the OOM killer is making progress */ >>> - if (did_some_progress) >>> + if (did_some_progress) { >>> + stall_backoff = 0; >>> goto retry; >>> + } >> >> Umm ? I'm sorry that I didn't notice page allocation may fail even >> if order < PAGE_ALLOC_COSTLY_ORDER. I thought old logic ignores >> did_some_progress. It seems a big change. > > __alloc_pages_may_oom will set did_some_progress > >> So, now, 0-order page allocation may fail in a OOM situation ? > > No they don't normally and this patch doesn't change the logic here. > I understand your patch doesn't change the behavior. Looking into __alloc_pages_may_oom(), *did_some_progress is finally set by if (out_of_memory(&oc) || WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL)) *did_some_progress = 1; ...depends on out_of_memory() return value. Now, allocation may fail if oom-killer is disabled.... Isn't it complicated ? Shouldn't we have if (order < PAGE_ALLOC_COSTLY_ORDER) goto retry; here ? Thanks, -Kame