linux-kernel - [RFC] vmscan: initialize sc->nr_reclaimed in do_try_to_free

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-next>] [day] [month] [year] [list]

Date:	Mon, 9 Feb 2009 23:24:16 +0100
From:	Johannes Weiner <hannes@...xchg.org>
To:	Rik van Riel <riel@...hat.com>
Cc:	William Lee Irwin III <wli@...ementarian.org>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: [RFC] vmscan: initialize sc->nr_reclaimed in do_try_to_free_pages()

Commit a79311c14eae4bb946a97af25f3e1b17d625985d "vmscan: bail out of
direct reclaim after swap_cluster_max pages" moved the nr_reclaimed
counter into the scan control to accumulate the number of all
reclaimed pages in one direct reclaim invocation.

The commit missed to actually adjust do_try_to_free_pages() which now
does not initialize sc.nr_reclaimed and makes shrink_zone() make
assumptions on whether to bail out of the reclaim cycle based on an
uninitialized value.

Fix it up by initializing the counter to zero before entering the
priority loop.

Signed-off-by: Johannes Weiner <hannes@...xchg.org>
---
 mm/vmscan.c |    1 +
 1 file changed, 1 insertion(+)

The comment of the .nr_reclaimed field says it accumulates the reclaim
counter over ONE shrink_zones() call.  This means, we should break out
if ONE shrink_zones() call alone does more than swap_cluster_max.

OTOH, the patch title suggests that we break out if ALL shrink_zones()
calls in the priority loop have reclaimed that much.  I.e.
accumulating the reclaimed number over the prio loop, not just over
one zones iteration.

>From the patch description I couldn't really make sure what the
intended behaviour was.

So, should the sc.nr_reclaimed be reset before the prio loop or in
each iteration of the prio loop?

Either this patch is wrong or the comment above .nr_reclaimed is.

And why didn't this have any observable effects?  Do I miss something
really obvious here?

--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1618,6 +1618,7 @@ static unsigned long do_try_to_free_page
 		}
 	}

+	sc->nr_reclaimed = 0;
 	for (priority = DEF_PRIORITY; priority >= 0; priority--) {
 		sc->nr_scanned = 0;
 		if (!priority)
--
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/