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

From: Hugh Dickins
Date: Sun Jul 22 2012 - 15:25:54 EST


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>
>