linux-kernel - Re: [PATCH -mm 09/15] add some sanity checks to get_scan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 15 May 2008 15:34:51 +0900
From:	"MinChan Kim" <minchan.kim@...il.com>
To:	"Rik van Riel" <riel@...hat.com>
Cc:	linux-kernel@...r.kernel.org, lee.schermerhorn@...com,
	akpm@...ux-foundation.org, kosaki.motohiro@...fujitsu.com
Subject: Re: [PATCH -mm 09/15] add some sanity checks to get_scan_ratio

> @@ -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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/