Re: [PATCH 21/34] kswapd: assign new_order and new_classzone_idxafter wakeup in sleeping

[Hugh Dickins]

On Thu, 19 Jul 2012, Mel Gorman wrote:
> From: "Alex,Shi" <alex.shi@xxxxxxxxx>
> 
> commit d2ebd0f6b89567eb93ead4e2ca0cbe03021f344b upstream.

Thanks for assembling these, Mel: I was checking through to see if
I was missing any, and noticed that this one has the wrong upstream
SHA1: the one you give here is the same as in 20/34, but it should be

commit f0dfcde099453aa4c0dc42473828d15a6d492936 upstream.

I got quite confused by 30/34 too: interesting definition of "partial
backport" :) I've no objection, but "substitute" might be clearer there.

Hugh

> 
> Stable note: Fixes https://bugzilla.redhat.com/show_bug.cgi?id=712019. This
> 	patch reduces kswapd CPU usage.
> 
> There 2 places to read pgdat in kswapd.  One is return from a successful
> balance, another is waked up from kswapd sleeping.  The new_order and
> new_classzone_idx represent the balance input order and classzone_idx.
> 
> But current new_order and new_classzone_idx are not assigned after
> kswapd_try_to_sleep(), that will cause a bug in the following scenario.
> 
> 1: after a successful balance, kswapd goes to sleep, and new_order = 0;
>    new_classzone_idx = __MAX_NR_ZONES - 1;
> 
> 2: kswapd waked up with order = 3 and classzone_idx = ZONE_NORMAL
> 
> 3: in the balance_pgdat() running, a new balance wakeup happened with
>    order = 5, and classzone_idx = ZONE_NORMAL
> 
> 4: the first wakeup(order = 3) finished successufly, return order = 3
>    but, the new_order is still 0, so, this balancing will be treated as a
>    failed balance.  And then the second tighter balancing will be missed.
> 
> So, to avoid the above problem, the new_order and new_classzone_idx need
> to be assigned for later successful comparison.
> 
> Signed-off-by: Alex Shi <alex.shi@xxxxxxxxx>
> Acked-by: Mel Gorman <mgorman@xxxxxxx>
> Reviewed-by: Minchan Kim <minchan.kim@xxxxxxxxx>
> Tested-by: PÃdraig Brady <P@xxxxxxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Mel Gorman <mgorman@xxxxxxx>
> ---
>  mm/vmscan.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bf85e4d..b8c1fc0 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2905,6 +2905,8 @@ static int kswapd(void *p)
>  						balanced_classzone_idx);
>  			order = pgdat->kswapd_max_order;
>  			classzone_idx = pgdat->classzone_idx;
> +			new_order = order;
> +			new_classzone_idx = classzone_idx;
>  			pgdat->kswapd_max_order = 0;
>  			pgdat->classzone_idx = pgdat->nr_zones - 1;
>  		}
> -- 
> 1.7.9.2
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
>