Re: [PATCH -mm 09/15] add some sanity checks to get_scan_ratio

["MinChan Kim" <minchan.kim@gmail.com>]

> @@ -1256,7 +1285,7 @@ static unsigned long shrink_zone(int pri
>        }
>
>        while (nr[LRU_INACTIVE_ANON] || nr[LRU_ACTIVE_FILE] ||
> -                                                nr[LRU_INACTIVE_FILE]) {
> +                                       nr[LRU_INACTIVE_FILE]) {
>                for_each_lru(l) {
>                        if (nr[l]) {
>                                nr_to_scan = min(nr[l],
> @@ -1269,6 +1298,14 @@ static unsigned long shrink_zone(int pri
>                }
>        }
>
> +       /*
> +        * Even if we did not try to evict anon pages at all, we want to
> +        * rebalance the anon lru active/inactive ratio.
> +        */
> +       if (scan_global_lru(sc) && inactive_anon_low(zone))
> +               shrink_list(NR_ACTIVE_ANON, SWAP_CLUSTER_MAX, zone, sc,
> +                                                               priority);
> +
>        throttle_vm_writeout(sc->gfp_mask);
>        return nr_reclaimed;
>  }

I think It's rather typo error.
If it is error, It will cause wrong algorithm.

--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1375,7 +1375,7 @@ static unsigned long shrink_zone(int priority, struct zon
         * rebalance the anon lru active/inactive ratio.
         */
        if (scan_global_lru(sc) && inactive_anon_low(zone))
-               shrink_list(NR_ACTIVE_ANON, SWAP_CLUSTER_MAX, zone, sc,
+               shrink_list(LRU_ACTIVE_ANON, SWAP_CLUSTER_MAX, zone, sc,
                                                                priority);

        throttle_vm_writeout(sc->gfp_mask);



shrink_list called twice about LRU_ACTIVE_ANON if
(nr[LRU_INACTIVE_ANON] != 0 && inactive_anon_low(zone))
Is it your intention ? You want to put the pressure twice active anon
list on above condition ?

If your intention is right, I think following code about readability
is good than old.

--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1375,8 +1375,7 @@ static unsigned long shrink_zone(int priority, struct zon
         * rebalance the anon lru active/inactive ratio.
         */
        if (scan_global_lru(sc) && inactive_anon_low(zone))
-               shrink_list(NR_ACTIVE_ANON, SWAP_CLUSTER_MAX, zone, sc,
-                                                               priority);
+               shrink_inactive_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);

        throttle_vm_writeout(sc->gfp_mask);
        return nr_reclaimed;


Gmail client will mangle my patch. This is just purpose of review.

-- 
Kinds regards,
MinChan Kim