Re: [PATCH 10/27] mm, vmscan: Clear congestion, dirty and need for compaction on a per-node basis

Vlastimil Babka
2016-06-16T09:29:10Z

On 06/09/2016 08:04 PM, Mel Gorman wrote:
> Congested and dirty tracking of a node and whether reclaim should stall
> is still based on zone activity. This patch considers whether the kernel
> should stall based on node-based reclaim activity.

I'm a bit confused about the description vs actual code.
It appears to move some duplicated code to a related function, which is
fine. The rest of callsites that didn't perform the clearing before
(prepare_kswapd_sleep() and wakeup_kswapd()) might be a bit overkill,
but won't hurt. But I don't see the part "considers whether the kernel
should stall based on node-based reclaim activity". Is something missing?

> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> ---
> mm/vmscan.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index dd68e3154732..e4f3e068b7a0 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2966,7 +2966,17 @@ static bool zone_balanced(struct zone *zone, int order, int classzone_idx)
> {
> unsigned long mark = high_wmark_pages(zone);
>
> - return zone_watermark_ok_safe(zone, order, mark, classzone_idx);
> + if (!zone_watermark_ok_safe(zone, order, mark, classzone_idx))
> + return false;
> +
> + /*
> + * If any eligible zone is balanced then the node is not considered
> + * to be congested or dirty
> + */
> + clear_bit(PGDAT_CONGESTED, &zone->zone_pgdat->flags);
> + clear_bit(PGDAT_DIRTY, &zone->zone_pgdat->flags);
> +
> + return true;
> }
>
> /*
> @@ -3112,13 +3122,6 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int classzone_idx)
> if (!zone_balanced(zone, order, 0)) {
> classzone_idx = i;
> break;
> - } else {
> - /*
> - * If any eligible zone is balanced then the
> - * node is not considered congested or dirty.
> - */
> - clear_bit(PGDAT_CONGESTED, &zone->zone_pgdat->flags);
> - clear_bit(PGDAT_DIRTY, &zone->zone_pgdat->flags);
> }
> }
>
> @@ -3177,11 +3180,8 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int classzone_idx)
> if (!populated_zone(zone))
> continue;
>
> - if (zone_balanced(zone, sc.order, classzone_idx)) {
> - clear_bit(PGDAT_CONGESTED, &pgdat->flags);
> - clear_bit(PGDAT_DIRTY, &pgdat->flags);
> + if (zone_balanced(zone, sc.order, classzone_idx))
> goto out;
> - }
> }
>
> /*
>