Re: [PATCH 34/34] mm, vmstat: remove zone and node double accounting by approximating retries

[Mel Gorman]

On Thu, Jul 14, 2016 at 03:40:11PM +0200, Vlastimil Babka wrote:
> >@@ -4,6 +4,26 @@
> > #include <linux/huge_mm.h>
> > #include <linux/swap.h>
> >
> >+#ifdef CONFIG_HIGHMEM
> >+extern atomic_t highmem_file_pages;
> >+
> >+static inline void acct_highmem_file_pages(int zid, enum lru_list lru,
> >+							int nr_pages)
> >+{
> >+	if (is_highmem_idx(zid) && is_file_lru(lru)) {
> >+		if (nr_pages > 0)
> 
> This seems like a unnecessary branch, atomic_add should handle negative
> nr_pages just fine?
> 

On x86 it would but the interface makes no guarantees it'll handle
signed types properly on all architectures.

> >@@ -1456,14 +1461,27 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,
> > 		unsigned long available;
> > 		enum compact_result compact_result;
> >
> >+		if (last_pgdat == zone->zone_pgdat)
> >+			continue;
> >+
> >+		/*
> >+		 * This over-estimates the number of pages available for
> >+		 * reclaim/compaction but walking the LRU would take too
> >+		 * long. The consequences are that compaction may retry
> >+		 * longer than it should for a zone-constrained allocation
> >+		 * request.
> 
> The comment above says that we don't retry zone-constrained at all. Is this
> an obsolete comment, or does it refer to the ZONE_NORMAL constraint? (as
> opposed to HIGHMEM, MOVABLE etc?).
> 

It can still over-estimate the amount of memory available if
ZONE_MOVABLE exists even if the request is not zone-constrained.

> >@@ -3454,6 +3455,15 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
> > 		return false;
> >
> > 	/*
> >+	 * Blindly retry lowmem allocation requests that are often ignored by
> >+	 * the OOM killer up to MAX_RECLAIM_RETRIES as we not have a reliable
> >+	 * and fast means of calculating reclaimable, dirty and writeback pages
> >+	 * in eligible zones.
> >+	 */
> >+	if (ac->high_zoneidx < ZONE_NORMAL)
> >+		goto out;
> 
> A goto inside two nested for cycles? Is there no hope for sanity? :(
> 

None, hand it in at the door.

It can be pulled out and put past the "return false" at the end. It's
just not necessarily any better.

-- 
Mel Gorman
SUSE Labs