lkml.org 
[lkml]   [2016]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 34/34] mm, vmstat: remove zone and node double accounting by approximating retries
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

\
 
 \ /
  Last update: 2016-07-15 10:21    [W:0.062 / U:26.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site