From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932845AbbKRPT3 (ORCPT ); Wed, 18 Nov 2015 10:19:29 -0500 Received: from mx2.suse.de ([195.135.220.15]:47120 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755980AbbKRPTR (ORCPT ); Wed, 18 Nov 2015 10:19:17 -0500 Subject: Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without triggering reclaim To: Michal Hocko References: <1447680139-16484-1-git-send-email-mhocko@kernel.org> <1447680139-16484-3-git-send-email-mhocko@kernel.org> <564C91E9.8000904@suse.cz> <20151118151119.GG19145@dhcp22.suse.cz> Cc: Andrew Morton , Mel Gorman , David Rientjes , linux-mm@kvack.org, LKML From: Vlastimil Babka Message-ID: <564C96F2.10102@suse.cz> Date: Wed, 18 Nov 2015 16:19:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151118151119.GG19145@dhcp22.suse.cz> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/18/2015 04:11 PM, Michal Hocko wrote: > On Wed 18-11-15 15:57:45, Vlastimil Babka wrote: > [...] >> > --- a/mm/page_alloc.c >> > +++ b/mm/page_alloc.c >> > @@ -3046,32 +3046,36 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, >> > * allocations are system rather than user orientated >> > */ >> > ac->zonelist = node_zonelist(numa_node_id(), gfp_mask); >> > - do { >> > - page = get_page_from_freelist(gfp_mask, order, >> > - ALLOC_NO_WATERMARKS, ac); >> > - if (page) >> > - goto got_pg; >> > - >> > - if (gfp_mask & __GFP_NOFAIL) >> > - wait_iff_congested(ac->preferred_zone, >> > - BLK_RW_ASYNC, HZ/50); >> >> I've been thinking if the lack of unconditional wait_iff_congested() can affect >> something negatively. I guess not? > > Considering that the wait_iff_congested is removed only for PF_MEMALLOC > with __GFP_NOFAIL which should be non-existent in the kernel then I Hm that one won't reach it indeed, but also not loop, so that wasn't my concern. I was referring to: /* Keep reclaiming pages as long as there is reasonable progress */ 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; } Here we might skip the wait_iff_congested and go straight for oom. But it's true that ordinary allocations that fail to make progress will also not wait, so I guess it's fine. Acked-by: Vlastimil Babka > think the risk is really low. Even if there was a caller _and_ there > was a congestion then the behavior wouldn't be much more worse than > what we have currently. The system is out of memory hoplessly if > ALLOC_NO_WATERMARKS allocation fails. >