All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Linux-MM <linux-mm@kvack.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Rik van Riel <riel@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Michal Hocko <mhocko@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 01/10] mm, page_alloc: Delete the zonelist_cache
Date: Thu, 20 Aug 2015 15:17:20 +0100	[thread overview]
Message-ID: <20150820141720.GE12432@techsingularity.net> (raw)
In-Reply-To: <55D5D68E.6040206@suse.cz>

On Thu, Aug 20, 2015 at 03:30:54PM +0200, Vlastimil Babka wrote:
> >Note the maximum stall latency which was 6 seconds and becomes 67ms with
> >this patch applied. However, also note that it is not guaranteed this
> >benchmark always hits pathelogical cases and the milage varies. There is
> >a secondary impact with more direct reclaim because zones are now being
> >considered instead of being skipped by zlc.
> >
> >                                  4.1.0       4.1.0
> >                                vanilla  nozlc-v1r4
> >Swap Ins                           838         502
> >Swap Outs                      1149395     2622895
> >DMA32 allocs                  17839113    15863747
> >Normal allocs                129045707   137847920
> >Direct pages scanned           4070089    29046893
> >Kswapd pages scanned          17147837    17140694
> >Kswapd pages reclaimed        17146691    17139601
> >Direct pages reclaimed         1888879     4886630
> >Kswapd efficiency                  99%         99%
> >Kswapd velocity              17523.721   17518.928
> >Direct efficiency                  46%         16%
> >Direct velocity               4159.306   29687.854
> >Percentage direct scans            19%         62%
> >Page writes by reclaim     1149395.000 2622895.000
> >Page writes file                     0           0
> >Page writes anon               1149395     2622895
> 
> Interesting, kswapd has no decrease that would counter the increase in
> direct reclaim. So there's more reclaim overall. Does it mean that stutter
> doesn't like LRU and zlc was disrupting LRU?
> 

The LRU is being heavily disrupted by both reclaim and compaction
activity. The test is not a reliable means of evaluating reclaim decisions
because of the compaction activity. The main purpose of stutter was as a
proxy measure of desktop interactivity during IO.

As the test does THP allocations, it can trigger the case where zlc can
disable a zone for no reason and instead busy loop which is just wrong.

> >The direct page scan and reclaim rates are noticeable. It is possible
> >this will not be a universal win on all workloads but cycling through
> >zonelists waiting for zlc->last_full_zap to expire is not the right
> >decision.
> >
> >Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> >Acked-by: David Rientjes <rientjes@google.com>
> 
> It doesn't seem that removal of zlc would increase overhead due to
> "expensive operations no longer being avoided". Making some corner-case
> benchmark(s) worse as a side-effect of different LRU approximation shouldn't
> be a show-stopper. Hence
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> 

Thanks.

> just git grep found some lines that should be also deleted:
> 
> include/linux/mmzone.h: * If zlcache_ptr is not NULL, then it is just the
> address of zlcache,
> include/linux/mmzone.h: * as explained above.  If zlcache_ptr is NULL, there
> is no zlcache.
> 

Thanks

> And:
> 
> >@@ -3157,7 +2967,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> >  	gfp_t alloc_mask; /* The gfp_t that was actually used for allocation */
> >  	struct alloc_context ac = {
> >  		.high_zoneidx = gfp_zone(gfp_mask),
> >-		.nodemask = nodemask,
> >+		.nodemask = nodemask ? : &cpuset_current_mems_allowed,
> >  		.migratetype = gfpflags_to_migratetype(gfp_mask),
> >  	};
> >
> >@@ -3188,8 +2998,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> >  	ac.zonelist = zonelist;
> >  	/* The preferred zone is used for statistics later */
> >  	preferred_zoneref = first_zones_zonelist(ac.zonelist, ac.high_zoneidx,
> >-				ac.nodemask ? : &cpuset_current_mems_allowed,
> >-				&ac.preferred_zone);
> >+				ac.nodemask, &ac.preferred_zone);
> >  	if (!ac.preferred_zone)
> >  		goto out;
> >  	ac.classzone_idx = zonelist_zone_idx(preferred_zoneref);
> 
> These hunks appear unrelated to zonelist cache? Also they move the
> evaluation of cpuset_current_mems_allowed

They are rebase-related brain damage :(. I'll fix it and retest.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@techsingularity.net>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Linux-MM <linux-mm@kvack.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Rik van Riel <riel@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Michal Hocko <mhocko@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 01/10] mm, page_alloc: Delete the zonelist_cache
Date: Thu, 20 Aug 2015 15:17:20 +0100	[thread overview]
Message-ID: <20150820141720.GE12432@techsingularity.net> (raw)
In-Reply-To: <55D5D68E.6040206@suse.cz>

On Thu, Aug 20, 2015 at 03:30:54PM +0200, Vlastimil Babka wrote:
> >Note the maximum stall latency which was 6 seconds and becomes 67ms with
> >this patch applied. However, also note that it is not guaranteed this
> >benchmark always hits pathelogical cases and the milage varies. There is
> >a secondary impact with more direct reclaim because zones are now being
> >considered instead of being skipped by zlc.
> >
> >                                  4.1.0       4.1.0
> >                                vanilla  nozlc-v1r4
> >Swap Ins                           838         502
> >Swap Outs                      1149395     2622895
> >DMA32 allocs                  17839113    15863747
> >Normal allocs                129045707   137847920
> >Direct pages scanned           4070089    29046893
> >Kswapd pages scanned          17147837    17140694
> >Kswapd pages reclaimed        17146691    17139601
> >Direct pages reclaimed         1888879     4886630
> >Kswapd efficiency                  99%         99%
> >Kswapd velocity              17523.721   17518.928
> >Direct efficiency                  46%         16%
> >Direct velocity               4159.306   29687.854
> >Percentage direct scans            19%         62%
> >Page writes by reclaim     1149395.000 2622895.000
> >Page writes file                     0           0
> >Page writes anon               1149395     2622895
> 
> Interesting, kswapd has no decrease that would counter the increase in
> direct reclaim. So there's more reclaim overall. Does it mean that stutter
> doesn't like LRU and zlc was disrupting LRU?
> 

The LRU is being heavily disrupted by both reclaim and compaction
activity. The test is not a reliable means of evaluating reclaim decisions
because of the compaction activity. The main purpose of stutter was as a
proxy measure of desktop interactivity during IO.

As the test does THP allocations, it can trigger the case where zlc can
disable a zone for no reason and instead busy loop which is just wrong.

> >The direct page scan and reclaim rates are noticeable. It is possible
> >this will not be a universal win on all workloads but cycling through
> >zonelists waiting for zlc->last_full_zap to expire is not the right
> >decision.
> >
> >Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> >Acked-by: David Rientjes <rientjes@google.com>
> 
> It doesn't seem that removal of zlc would increase overhead due to
> "expensive operations no longer being avoided". Making some corner-case
> benchmark(s) worse as a side-effect of different LRU approximation shouldn't
> be a show-stopper. Hence
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> 

Thanks.

> just git grep found some lines that should be also deleted:
> 
> include/linux/mmzone.h: * If zlcache_ptr is not NULL, then it is just the
> address of zlcache,
> include/linux/mmzone.h: * as explained above.  If zlcache_ptr is NULL, there
> is no zlcache.
> 

Thanks

> And:
> 
> >@@ -3157,7 +2967,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> >  	gfp_t alloc_mask; /* The gfp_t that was actually used for allocation */
> >  	struct alloc_context ac = {
> >  		.high_zoneidx = gfp_zone(gfp_mask),
> >-		.nodemask = nodemask,
> >+		.nodemask = nodemask ? : &cpuset_current_mems_allowed,
> >  		.migratetype = gfpflags_to_migratetype(gfp_mask),
> >  	};
> >
> >@@ -3188,8 +2998,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> >  	ac.zonelist = zonelist;
> >  	/* The preferred zone is used for statistics later */
> >  	preferred_zoneref = first_zones_zonelist(ac.zonelist, ac.high_zoneidx,
> >-				ac.nodemask ? : &cpuset_current_mems_allowed,
> >-				&ac.preferred_zone);
> >+				ac.nodemask, &ac.preferred_zone);
> >  	if (!ac.preferred_zone)
> >  		goto out;
> >  	ac.classzone_idx = zonelist_zone_idx(preferred_zoneref);
> 
> These hunks appear unrelated to zonelist cache? Also they move the
> evaluation of cpuset_current_mems_allowed

They are rebase-related brain damage :(. I'll fix it and retest.

-- 
Mel Gorman
SUSE Labs

  reply	other threads:[~2015-08-20 14:17 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-12 10:45 [PATCH 00/10] Remove zonelist cache and high-order watermark checking v2 Mel Gorman
2015-08-12 10:45 ` Mel Gorman
2015-08-12 10:45 ` [PATCH 01/10] mm, page_alloc: Delete the zonelist_cache Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-20 13:18   ` Michal Hocko
2015-08-20 13:18     ` Michal Hocko
2015-08-20 13:42     ` Mel Gorman
2015-08-20 13:42       ` Mel Gorman
2015-08-21  9:29       ` Michal Hocko
2015-08-21  9:29         ` Michal Hocko
2015-08-20 13:30   ` Vlastimil Babka
2015-08-20 13:30     ` Vlastimil Babka
2015-08-20 14:17     ` Mel Gorman [this message]
2015-08-20 14:17       ` Mel Gorman
2015-08-20 14:45       ` Vlastimil Babka
2015-08-20 14:45         ` Vlastimil Babka
2015-08-12 10:45 ` [PATCH 02/10] mm, page_alloc: Remove unnecessary parameter from zone_watermark_ok_safe Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-20 12:30   ` Michal Hocko
2015-08-20 12:30     ` Michal Hocko
2015-08-12 10:45 ` [PATCH 03/10] mm, page_alloc: Remove unnecessary recalculations for dirty zone balancing Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-20 12:45   ` Michal Hocko
2015-08-20 12:45     ` Michal Hocko
2015-08-20 13:45     ` Mel Gorman
2015-08-20 13:45       ` Mel Gorman
2015-08-20 14:25       ` Michal Hocko
2015-08-20 14:25         ` Michal Hocko
2015-08-12 10:45 ` [PATCH 04/10] mm, page_alloc: Remove unnecessary taking of a seqlock when cpusets are disabled Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-13  0:16   ` David Rientjes
2015-08-13  0:16     ` David Rientjes
2015-08-17 11:58     ` Mel Gorman
2015-08-17 11:58       ` Mel Gorman
2015-08-12 10:45 ` [PATCH 05/10] mm, page_alloc: Use masks and shifts when converting GFP flags to migrate types Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-12 14:45   ` Christoph Lameter
2015-08-12 14:45     ` Christoph Lameter
2015-08-12 10:45 ` [PATCH 06/10] mm: page_alloc: Distinguish between being unable to sleep, unwilling to unwilling and avoiding waking kswapd Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-12 13:22   ` Michal Hocko
2015-08-12 13:22     ` Michal Hocko
2015-08-19 14:44   ` Vlastimil Babka
2015-08-19 14:44     ` Vlastimil Babka
2015-08-20  9:14     ` Mel Gorman
2015-08-20  9:14       ` Mel Gorman
2015-08-21 13:42   ` Vlastimil Babka
2015-08-21 13:42     ` Vlastimil Babka
2015-08-21 20:39     ` Mel Gorman
2015-08-21 20:39       ` Mel Gorman
2015-08-12 10:45 ` [PATCH 07/10] mm: page_alloc: Rename __GFP_WAIT to __GFP_RECLAIM Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-20 12:28   ` Michal Hocko
2015-08-20 12:28     ` Michal Hocko
2015-08-20 12:46     ` Michal Hocko
2015-08-20 12:46       ` Michal Hocko
2015-08-21 14:20   ` Vlastimil Babka
2015-08-21 14:20     ` Vlastimil Babka
2015-08-21 20:56     ` Mel Gorman
2015-08-21 20:56       ` Mel Gorman
2015-08-12 10:45 ` [PATCH 08/10] mm, page_alloc: Remove MIGRATE_RESERVE Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-12 10:45 ` [PATCH 09/10] mm, page_alloc: Reserve pageblocks for high-order atomic allocations on demand Mel Gorman
2015-08-12 10:45   ` Mel Gorman
2015-08-12 10:45 ` [PATCH 10/10] mm, page_alloc: Only enforce watermarks for order-0 allocations Mel Gorman
2015-08-12 10:45   ` Mel Gorman
  -- strict thread matches above, loose matches on Subject: below --
2015-07-20  8:00 [RFC PATCH 00/10] Remove zonelist cache and high-order watermark checking Mel Gorman
2015-07-20  8:00 ` [PATCH 01/10] mm, page_alloc: Delete the zonelist_cache Mel Gorman
2015-07-20  8:00   ` Mel Gorman
2015-07-21 23:47   ` David Rientjes
2015-07-21 23:47     ` David Rientjes
2015-07-23 10:58     ` Mel Gorman
2015-07-23 10:58       ` Mel Gorman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150820141720.GE12432@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=hannes@cmpxchg.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.