Re: [PATCH 01/31] mm, vmstat: add infrastructure for per-node vmstats

[Mel Gorman]

On Tue, Jul 05, 2016 at 08:50:18AM +0900, Minchan Kim wrote:
> > @@ -172,13 +174,17 @@ void refresh_zone_stat_thresholds(void)
> >  	int threshold;
> >  
> >  	for_each_populated_zone(zone) {
> > +		struct pglist_data *pgdat = zone->zone_pgdat;
> >  		unsigned long max_drift, tolerate_drift;
> >  
> >  		threshold = calculate_normal_threshold(zone);
> >  
> > -		for_each_online_cpu(cpu)
> > +		for_each_online_cpu(cpu) {
> >  			per_cpu_ptr(zone->pageset, cpu)->stat_threshold
> >  							= threshold;
> > +			per_cpu_ptr(pgdat->per_cpu_nodestats, cpu)->stat_threshold
> > +							= threshold;
> > +		}
> 
> I didn't see other patches yet so it might fix it then.
> 
> per_cpu_nodestats is per node not zone but it use per-zone threshold
> and even overwritten by next zones. I don't think it's not intended.

It was intended that the threshold from one zone would be used but now
that you point it out, it would use the threshold for the smallest zone
in the node which is sub-optimal. I applied the patch below on top to
use the threshold from the largest zone. I considered using the sum of
all thresholds but feared it might allow too much per-cpu drift. It can
be switched to the sum if we find a case where vmstat updates are too
high.

diff --git a/mm/vmstat.c b/mm/vmstat.c
index 90b0737ee4be..3345d396a99b 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -169,10 +169,18 @@ int calculate_normal_threshold(struct zone *zone)
  */
 void refresh_zone_stat_thresholds(void)
 {
+	struct pglist_data *pgdat;
 	struct zone *zone;
 	int cpu;
 	int threshold;
 
+	/* Zero current pgdat thresholds */
+	for_each_online_pgdat(pgdat) {
+		for_each_online_cpu(cpu) {
+			per_cpu_ptr(pgdat->per_cpu_nodestats, cpu)->stat_threshold = 0;
+		}
+	}
+
 	for_each_populated_zone(zone) {
 		struct pglist_data *pgdat = zone->zone_pgdat;
 		unsigned long max_drift, tolerate_drift;
@@ -180,10 +188,15 @@ void refresh_zone_stat_thresholds(void)
 		threshold = calculate_normal_threshold(zone);
 
 		for_each_online_cpu(cpu) {
+			int pgdat_threshold;
+
 			per_cpu_ptr(zone->pageset, cpu)->stat_threshold
 							= threshold;
+
+			/* Base nodestat threshold on the largest populated zone. */
+			pgdat_threshold = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu)->stat_threshold;
 			per_cpu_ptr(pgdat->per_cpu_nodestats, cpu)->stat_threshold
-							= threshold;
+				= max(threshold, pgdat_threshold);
 		}
 
 		/*

-- 
Mel Gorman
SUSE Labs