[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160715074859.GM9806@techsingularity.net>
Date: Fri, 15 Jul 2016 08:48:59 +0100
From: Mel Gorman <mgorman@...hsingularity.net>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Linux-MM <linux-mm@...ck.org>, Rik van Riel <riel@...riel.com>,
Johannes Weiner <hannes@...xchg.org>,
Minchan Kim <minchan@...nel.org>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [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
Powered by blists - more mailing lists