linux-kernel - Re: [PATCH 2/2] mm, oom, compaction: prevent from should_compact

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160412124344.GE10771@dhcp22.suse.cz>
Date:	Tue, 12 Apr 2016 14:43:44 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	Vlastimil Babka <vbabka@...e.cz>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>, Rik van Riel <riel@...hat.com>,
	Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] mm, oom, compaction: prevent from
 should_compact_retry looping for ever for costly orders

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