Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1435591 > unrolled thread

[PATCH 20/31] mm, vmscan: only wakeup kswapd once per node for the requested classzone

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 20/31] mm, vmscan: only wakeup kswapd once per node for the requested classzone Mel Gorman <mgorman@techsingularity.net> - 2016-07-01 22:10 +0200
    Re: [PATCH 20/31] mm, vmscan: only wakeup kswapd once per node for  the requested classzone Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-07-07 03:30 +0200
      Re: [PATCH 20/31] mm, vmscan: only wakeup kswapd once per node for  the requested classzone Mel Gorman <mgorman@techsingularity.net> - 2016-07-07 13:00 +0200

#1435591 — [PATCH 20/31] mm, vmscan: only wakeup kswapd once per node for the requested classzone

kswapd is woken when zones are below the low watermark but the wakeup
decision is not taking the classzone into account.  Now that reclaim is
node-based, it is only required to wake kswapd once per node and only if
all zones are unbalanced for the requested classzone.

Note that one node might be checked multiple times if the zonelist is
ordered by node because there is no cheap way of tracking what nodes have
already been visited.  For zone-ordering, each node should be checked only
once.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/page_alloc.c |  8 ++++++--
 mm/vmscan.c     | 13 +++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 441f482bf9a2..2fe2fbb4f2ad 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3410,10 +3410,14 @@ static void wake_all_kswapds(unsigned int order, const struct alloc_context *ac)
 {
 	struct zoneref *z;
 	struct zone *zone;
+	pg_data_t *last_pgdat = NULL;
 
 	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist,
-						ac->high_zoneidx, ac->nodemask)
-		wakeup_kswapd(zone, order, ac_classzone_idx(ac));
+					ac->high_zoneidx, ac->nodemask) {
+		if (last_pgdat != zone->zone_pgdat)
+			wakeup_kswapd(zone, order, ac_classzone_idx(ac));
+		last_pgdat = zone->zone_pgdat;
+	}
 }
 
 static inline unsigned int
diff --git a/mm/vmscan.c b/mm/vmscan.c
index c1c8b77d8cb4..e02091be0e12 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3420,6 +3420,7 @@ static int kswapd(void *p)
 void wakeup_kswapd(struct zone *zone, int order, enum zone_type classzone_idx)
 {
 	pg_data_t *pgdat;
+	int z;
 
 	if (!populated_zone(zone))
 		return;
@@ -3433,8 +3434,16 @@ void wakeup_kswapd(struct zone *zone, int order, enum zone_type classzone_idx)
 	pgdat->kswapd_order = max(pgdat->kswapd_order, order);
 	if (!waitqueue_active(&pgdat->kswapd_wait))
 		return;
-	if (zone_balanced(zone, order, 0))
-		return;
+
+	/* Only wake kswapd if all zones are unbalanced */
+	for (z = 0; z <= classzone_idx; z++) {
+		zone = pgdat->node_zones + z;
+		if (!populated_zone(zone))
+			continue;
+
+		if (zone_balanced(zone, order, classzone_idx))
+			return;
+	}
 
 	trace_mm_vmscan_wakeup_kswapd(pgdat->node_id, zone_idx(zone), order);
 	wake_up_interruptible(&pgdat->kswapd_wait);
-- 
2.6.4

[toc] | [next] | [standalone]

#1438092 — Re: [PATCH 20/31] mm, vmscan: only wakeup kswapd once per node for the requested classzone

On Fri, Jul 01, 2016 at 09:01:28PM +0100, Mel Gorman wrote:
> kswapd is woken when zones are below the low watermark but the wakeup
> decision is not taking the classzone into account.  Now that reclaim is
> node-based, it is only required to wake kswapd once per node and only if
> all zones are unbalanced for the requested classzone.
> 
> Note that one node might be checked multiple times if the zonelist is
> ordered by node because there is no cheap way of tracking what nodes have
> already been visited.  For zone-ordering, each node should be checked only
> once.
> 
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  mm/page_alloc.c |  8 ++++++--
>  mm/vmscan.c     | 13 +++++++++++--
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 441f482bf9a2..2fe2fbb4f2ad 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3410,10 +3410,14 @@ static void wake_all_kswapds(unsigned int order, const struct alloc_context *ac)
>  {
>  	struct zoneref *z;
>  	struct zone *zone;
> +	pg_data_t *last_pgdat = NULL;
>  
>  	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist,
> -						ac->high_zoneidx, ac->nodemask)
> -		wakeup_kswapd(zone, order, ac_classzone_idx(ac));
> +					ac->high_zoneidx, ac->nodemask) {
> +		if (last_pgdat != zone->zone_pgdat)
> +			wakeup_kswapd(zone, order, ac_classzone_idx(ac));
> +		last_pgdat = zone->zone_pgdat;
> +	}
>  }

In wakeup_kswapd(), there is a check if it is a populated zone or not.
If first zone in node is not a populated zone, wakeup_kswapd() would be
skipped. Though, I'm not sure if zonelist can include a un-populated
zone. Perhaps, moving populated zone check in wakeup_kswapd() to here
would be a safe code.

Thanks.

>  
>  static inline unsigned int
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index c1c8b77d8cb4..e02091be0e12 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3420,6 +3420,7 @@ static int kswapd(void *p)
>  void wakeup_kswapd(struct zone *zone, int order, enum zone_type classzone_idx)
>  {
>  	pg_data_t *pgdat;
> +	int z;
>  
>  	if (!populated_zone(zone))
>  		return;
> @@ -3433,8 +3434,16 @@ void wakeup_kswapd(struct zone *zone, int order, enum zone_type classzone_idx)
>  	pgdat->kswapd_order = max(pgdat->kswapd_order, order);
>  	if (!waitqueue_active(&pgdat->kswapd_wait))
>  		return;
> -	if (zone_balanced(zone, order, 0))
> -		return;
> +
> +	/* Only wake kswapd if all zones are unbalanced */
> +	for (z = 0; z <= classzone_idx; z++) {
> +		zone = pgdat->node_zones + z;
> +		if (!populated_zone(zone))
> +			continue;
> +
> +		if (zone_balanced(zone, order, classzone_idx))
> +			return;
> +	}
>  
>  	trace_mm_vmscan_wakeup_kswapd(pgdat->node_id, zone_idx(zone), order);
>  	wake_up_interruptible(&pgdat->kswapd_wait);
> -- 
> 2.6.4
> 
> --
> 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>

[toc] | [prev] | [next] | [standalone]

#1438521 — Re: [PATCH 20/31] mm, vmscan: only wakeup kswapd once per node for the requested classzone

On Thu, Jul 07, 2016 at 10:24:23AM +0900, Joonsoo Kim wrote:
> On Fri, Jul 01, 2016 at 09:01:28PM +0100, Mel Gorman wrote:
> > kswapd is woken when zones are below the low watermark but the wakeup
> > decision is not taking the classzone into account.  Now that reclaim is
> > node-based, it is only required to wake kswapd once per node and only if
> > all zones are unbalanced for the requested classzone.
> > 
> > Note that one node might be checked multiple times if the zonelist is
> > ordered by node because there is no cheap way of tracking what nodes have
> > already been visited.  For zone-ordering, each node should be checked only
> > once.
> > 
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > ---
> >  mm/page_alloc.c |  8 ++++++--
> >  mm/vmscan.c     | 13 +++++++++++--
> >  2 files changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 441f482bf9a2..2fe2fbb4f2ad 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -3410,10 +3410,14 @@ static void wake_all_kswapds(unsigned int order, const struct alloc_context *ac)
> >  {
> >  	struct zoneref *z;
> >  	struct zone *zone;
> > +	pg_data_t *last_pgdat = NULL;
> >  
> >  	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist,
> > -						ac->high_zoneidx, ac->nodemask)
> > -		wakeup_kswapd(zone, order, ac_classzone_idx(ac));
> > +					ac->high_zoneidx, ac->nodemask) {
> > +		if (last_pgdat != zone->zone_pgdat)
> > +			wakeup_kswapd(zone, order, ac_classzone_idx(ac));
> > +		last_pgdat = zone->zone_pgdat;
> > +	}
> >  }
> 
> In wakeup_kswapd(), there is a check if it is a populated zone or not.

It's redundant.

> If first zone in node is not a populated zone, wakeup_kswapd() would be
> skipped. Though, I'm not sure if zonelist can include a un-populated
> zone.

Zonelists do not contain unpopulated zones.

> Perhaps, moving populated zone check in wakeup_kswapd() to here
> would be a safe code.
> 

If anything was going to happen to it, it should be deleted. It's a
minor cleanup.

-- 
Mel Gorman
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web