From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932583AbcDLMns (ORCPT ); Tue, 12 Apr 2016 08:43:48 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:34233 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756815AbcDLMnr (ORCPT ); Tue, 12 Apr 2016 08:43:47 -0400 Date: Tue, 12 Apr 2016 14:43:44 +0200 From: Michal Hocko To: Vlastimil Babka Cc: Andrew Morton , Mel Gorman , Rik van Riel , Johannes Weiner , linux-mm@kvack.org, LKML Subject: Re: [PATCH 2/2] mm, oom, compaction: prevent from should_compact_retry looping for ever for costly orders Message-ID: <20160412124344.GE10771@dhcp22.suse.cz> References: <1460357151-25554-1-git-send-email-mhocko@kernel.org> <1460357151-25554-3-git-send-email-mhocko@kernel.org> <570CA287.3030604@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <570CA287.3030604@suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 12-04-16 09:23:51, Vlastimil Babka wrote: [...] > It's a bit complicated, but I agree that something like this is needed to > prevent unexpected endless loops. Alternatively you could maybe just extend > compact_result to distinguish between COMPACT_SKIPPED (but possible after > reclaim) and COMPACT_IMPOSSIBLE (or some better name?). Then > compaction_withdrawn() would obviously be false for IMPOSSIBLE, while > compaction_failed() would be true? Then you shouldn't need > compaction_zonelist_suitable(). I would rather not add more states. My head spins with the current state already... > >+bool compaction_zonelist_suitable(struct alloc_context *ac, int order, > >+ int alloc_flags) > >+{ > >+ struct zone *zone; > >+ struct zoneref *z; > >+ > >+ /* > >+ * Make sure at least one zone would pass __compaction_suitable if we continue > >+ * retrying the reclaim. > >+ */ > >+ for_each_zone_zonelist_nodemask(zone, z, ac->zonelist, ac->classzone_idx, > > I think here you should s/classzone_idx/high_zoneidx/ true > > >+ ac->nodemask) { > >+ unsigned long available; > >+ enum compact_result compact_result; > >+ > >+ /* > >+ * Do not consider all the reclaimable memory because we do not > >+ * want to trash just for a single high order allocation which > >+ * is even not guaranteed to appear even if __compaction_suitable > >+ * is happy about the watermark check. > >+ */ > >+ available = zone_reclaimable_pages(zone) / order; > >+ available += zone_page_state_snapshot(zone, NR_FREE_PAGES); > >+ compact_result = __compaction_suitable(zone, order, alloc_flags, > >+ ac->high_zoneidx, available); > > And vice versa here. will fix this. Thanks! -- Michal Hocko SUSE Labs