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

From: MinChan Kim
Date: Thu May 15 2008 - 02:35:07 EST


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