Re: [PATCH 0.14] oom detection rework v6

[Michal Hocko]

On Wed 04-05-16 14:45:02, Joonsoo Kim wrote:
> On Wed, Apr 20, 2016 at 03:47:13PM -0400, Michal Hocko wrote:
> > Hi,
> > 
> > This is v6 of the series. The previous version was posted [1]. The
> > code hasn't changed much since then. I have found one old standing
> > bug (patch 1) which just got much more severe and visible with this
> > series. Other than that I have reorganized the series and put the
> > compaction feedback abstraction to the front just in case we find out
> > that parts of the series would have to be reverted later on for some
> > reason. The premature oom killer invocation reported by Hugh [2] seems
> > to be addressed.
> > 
> > We have discussed this series at LSF/MM summit in Raleigh and there
> > didn't seem to be any concerns/objections to go on with the patch set
> > and target it for the next merge window. 
> 
> I still don't agree with some part of this patchset that deal with
> !costly order. As you know, there was two regression reports from Hugh
> and Aaron and you fixed them by ensuring to trigger compaction. I
> think that these show the problem of this patchset. Previous kernel
> doesn't need to ensure to trigger compaction and just works fine in
> any case. Your series make compaction necessary for all. OOM handling
> is essential part in MM but compaction isn't. OOM handling should not
> depend on compaction. I tested my own benchmark without
> CONFIG_COMPACTION and found that premature OOM happens.

High order allocations without compaction are basically a lost game. You
can wait unbounded amount of time and still have no guarantee of any
progress. What is the usual reason to disable compaction in the first
place?

Anyway if this is _really_ a big issue then we can do something like the
following to emulate the previous behavior. We are losing the
determinism but if you really thing that the !COMPACTION workloads
already reconcile with it I can live with that.
---
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2e7e26c5d3ba..f48b9e9b1869 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3319,6 +3319,24 @@ should_compact_retry(struct alloc_context *ac, unsigned int order, int alloc_fla
 		     enum migrate_mode *migrate_mode,
 		     int compaction_retries)
 {
+	struct zone *zone;
+	struct zoneref *z;
+
+	if (order > PAGE_ALLOC_COSTLY_ORDER)
+		return false;
+
+	/*
+	 * There are setups with compaction disabled which would prefer to loop
+	 * inside the allocator rather than hit the oom killer prematurely. Let's
+	 * give them a good hope and keep retrying while the order-0 watermarks
+	 * are OK.
+	 */
+	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist, ac->high_zoneidx,
+					ac->nodemask) {
+		if(zone_watermark_ok(zone, 0, min_wmark_pages(zone),
+					ac->high_zoneidx, alloc_flags))
+			return true;
+	}
 	return false;
 }
 #endif /* CONFIG_COMPACTION */
-- 
Michal Hocko
SUSE Labs