Re: [PATCH 1/5] mm: kswapd: Stop high-order balancing when anysuitable zone is balanced

[Minchan Kim]

Hi Mel,

On Fri, Dec 3, 2010 at 8:45 PM, Mel Gorman <mel@xxxxxxxxx> wrote:
> When the allocator enters its slow path, kswapd is woken up to balance the
> node. It continues working until all zones within the node are balanced. For
> order-0 allocations, this makes perfect sense but for higher orders it can
> have unintended side-effects. If the zone sizes are imbalanced, kswapd may
> reclaim heavily within a smaller zone discarding an excessive number of
> pages. The user-visible behaviour is that kswapd is awake and reclaiming
> even though plenty of pages are free from a suitable zone.
>
> This patch alters the "balance" logic for high-order reclaim allowing kswapd
> to stop if any suitable zone becomes balanced to reduce the number of pages
> it reclaims from other zones. kswapd still tries to ensure that order-0
> watermarks for all zones are met before sleeping.
>
> Signed-off-by: Mel Gorman <mel@xxxxxxxxx>

<snip>

> -       if (!all_zones_ok) {
> +       if (!(all_zones_ok || (order && any_zone_ok))) {
>                cond_resched();
>
>                try_to_freeze();
> @@ -2361,6 +2366,31 @@ out:
>                goto loop_again;
>        }
>
> +       /*
> +        * If kswapd was reclaiming at a higher order, it has the option of
> +        * sleeping without all zones being balanced. Before it does, it must
> +        * ensure that the watermarks for order-0 on *all* zones are met and
> +        * that the congestion flags are cleared
> +        */
> +       if (order) {
> +               for (i = 0; i <= end_zone; i++) {
> +                       struct zone *zone = pgdat->node_zones + i;
> +
> +                       if (!populated_zone(zone))
> +                               continue;
> +
> +                       if (zone->all_unreclaimable && priority != DEF_PRIORITY)
> +                               continue;
> +
> +                       zone_clear_flag(zone, ZONE_CONGESTED);

Why clear ZONE_CONGESTED?
If you have a cause, please, write down the comment.

<snip>

First impression on this patch is that it changes scanning behavior as
well as reclaiming on high order reclaim.
I can't say old behavior is right but we can't say this behavior is
right, too although this patch solves the problem. At least, we might
need some data that shows this patch doesn't have a regression. It's
not easy but I believe you can do very well as like having done until
now. I didn't see whole series so I might miss something.

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/