From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752942Ab1JYT3f (ORCPT ); Tue, 25 Oct 2011 15:29:35 -0400 Received: from smtp-out.google.com ([216.239.44.51]:35241 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752501Ab1JYT3e convert rfc822-to-8bit (ORCPT ); Tue, 25 Oct 2011 15:29:34 -0400 DomainKey-Signature: a=rsa-sha1; s=beta; d=google.com; c=nofws; q=dns; h=dkim-signature:mime-version:sender:in-reply-to:references:date: x-google-sender-auth:message-id:subject:from:to:cc:content-type: content-transfer-encoding:x-system-of-record; b=cmSyAIGLc/jY2w2odwU1R1RE4OCqeHKWI9/+JSQvpd+F73wKl9oLzlJTW5niKagVE qkGNhNfYJcp1IJ0T6kxtw== MIME-Version: 1.0 In-Reply-To: <20111025090956.GA10797@suse.de> References: <1319524789-22818-1-git-send-email-ccross@android.com> <20111025090956.GA10797@suse.de> Date: Tue, 25 Oct 2011 12:29:28 -0700 X-Google-Sender-Auth: WFq9LKhxLDbhF922HSZrAt5OYIs Message-ID: Subject: Re: [PATCH] mm: avoid livelock on !__GFP_FS allocations From: Colin Cross To: Mel Gorman Cc: linux-kernel@vger.kernel.org, Andrew Morton , KAMEZAWA Hiroyuki , Andrea Arcangeli , David Rientjes , linux-mm@kvack.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT X-System-Of-Record: true Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 25, 2011 at 2:09 AM, Mel Gorman wrote: > On Mon, Oct 24, 2011 at 11:39:49PM -0700, Colin Cross wrote: >> Under the following conditions, __alloc_pages_slowpath can loop >> forever: >> gfp_mask & __GFP_WAIT is true >> gfp_mask & __GFP_FS is false >> reclaim and compaction make no progress >> order <= PAGE_ALLOC_COSTLY_ORDER >> >> These conditions happen very often during suspend and resume, >> when pm_restrict_gfp_mask() effectively converts all GFP_KERNEL >> allocations into __GFP_WAIT. > b> >> The oom killer is not run because gfp_mask & __GFP_FS is false, >> but should_alloc_retry will always return true when order is less >> than PAGE_ALLOC_COSTLY_ORDER. >> >> Fix __alloc_pages_slowpath to skip retrying when oom killer is >> not allowed by the GFP flags, the same way it would skip if the >> oom killer was allowed but disabled. >> >> Signed-off-by: Colin Cross > > Hi Colin, > > Your patch functionally seems fine. I see the problem and we certainly > do not want to have the OOM killer firing during suspend. I would prefer > that the IO devices would not be suspended until reclaim was completed > but I imagine that would be a lot harder. > > That said, it will be difficult to remember why checking __GFP_NOFAIL in > this case is necessary and someone might "optimitise" it away later. It > would be preferable if it was self-documenting. Maybe something like > this? (This is totally untested) > >  mm/page_alloc.c |   22 ++++++++++++++++++++++ >  1 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 6e8ecb6..ad8f376 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -127,6 +127,20 @@ void pm_restrict_gfp_mask(void) >        saved_gfp_mask = gfp_allowed_mask; >        gfp_allowed_mask &= ~GFP_IOFS; >  } > + > +static bool pm_suspending(void) > +{ > +       if ((gfp_allowed_mask & GFP_IOFS) == GFP_IOFS) > +               return false; > +       return true; > +} > + > +#else > + > +static bool pm_suspending(void) > +{ > +       return false; > +} >  #endif /* CONFIG_PM_SLEEP */ > >  #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE > @@ -2207,6 +2221,14 @@ rebalance: > >                        goto restart; >                } > + > +               /* > +                * Suspend converts GFP_KERNEL to __GFP_WAIT which can > +                * prevent reclaim making forward progress without > +                * invoking OOM. Bail if we are suspending > +                */ > +               if (pm_suspending()) > +                       goto nopage; >        } > >        /* Check if we should retry the allocation */ > Your patch solves my immediate problem. Tested-by: Colin Cross