Re: [PATCH 2/2] mm, oom, compaction: prevent from should_compact_retry looping for ever for costly orders

[Michal Hocko]

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