diff mbox

[07/34] mm, vmscan: make kswapd reclaim in terms of nodes

Message ID 20160830150051.GW8119@techsingularity.net (mailing list archive)
State Not Applicable
Headers show

Commit Message

Mel Gorman Aug. 30, 2016, 3 p.m. UTC 
On Tue, Aug 30, 2016 at 07:55:08PM +0530, Srikar Dronamraju wrote:
> > > 
> > > This patch seems to hurt FA_DUMP functionality. This behaviour is not
> > > seen on v4.7 but only after this patch.
> > > 
> > > So when a kernel on a multinode machine with memblock_reserve() such
> > > that most of the nodes have zero available memory, kswapd seems to be
> > > consuming 100% of the time.
> > > 
> > 
> > Why is FA_DUMP specifically the trigger? If the nodes have zero available
> > memory then is the zone_populated() check failing when FA_DUMP is enabled? If
> > so, that would both allow kswapd to wake and stay awake.
> > 
> 
> The trigger is memblock_reserve() for the complete node memory.  And
> this is exactly what FA_DUMP does.  Here again the node has memory but
> its all reserved so there is no free memory in the node.
> 
> Did you mean populated_zone() when you said zone_populated or have I
> mistaken? populated_zone() does return 1 since it checks for
> zone->present_pages.
> 

Yes, I meant populated_zone(). Using present pages may have hidden
a long-lived corner case as it was unexpected that an entire node
would be reserved. The old code happened to survive *probably* because
pgdat_reclaimable would look false and kswapd checks for pgdat being
balanced would happen to do the right thing in this case.

Can you check if something like this works?

Comments

Srikar Dronamraju Aug. 31, 2016, 6:09 a.m. UTC | #1 
> > The trigger is memblock_reserve() for the complete node memory.  And
> > this is exactly what FA_DUMP does.  Here again the node has memory but
> > its all reserved so there is no free memory in the node.
> > 
> > Did you mean populated_zone() when you said zone_populated or have I
> > mistaken? populated_zone() does return 1 since it checks for
> > zone->present_pages.
> > 
> 
> Yes, I meant populated_zone(). Using present pages may have hidden
> a long-lived corner case as it was unexpected that an entire node
> would be reserved. The old code happened to survive *probably* because
> pgdat_reclaimable would look false and kswapd checks for pgdat being
> balanced would happen to do the right thing in this case.
> 
> Can you check if something like this works?
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index d572b78b65e1..cf64a5456cf6 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -830,7 +830,7 @@ unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
> 
>  static inline int populated_zone(struct zone *zone)
>  {
> -	return (!!zone->present_pages);
> +	return (!!zone->managed_pages);
>  }
> 
>  extern int movable_zone;
> 

This indeed fixes the problem.
Please add my 
Tested-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
diff mbox

Patch

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index d572b78b65e1..cf64a5456cf6 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -830,7 +830,7 @@  unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
 
 static inline int populated_zone(struct zone *zone)
 {
-	return (!!zone->present_pages);
+	return (!!zone->managed_pages);
 }
 
 extern int movable_zone;