linux-kernel - Re: [PATCH 4/11] ksm: reorganize ksm_check_stable

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20130205164823.GJ21389@suse.de>
Date:	Tue, 5 Feb 2013 16:48:23 +0000
From:	Mel Gorman <mgorman@...e.de>
To:	Hugh Dickins <hughd@...gle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Petr Holasek <pholasek@...hat.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Izik Eidus <izik.eidus@...ellosystems.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 4/11] ksm: reorganize ksm_check_stable_tree

On Fri, Jan 25, 2013 at 05:59:35PM -0800, Hugh Dickins wrote:
> Memory hotremove's ksm_check_stable_tree() is pitifully inefficient
> (restarting whenever it finds a stale node to remove), but rearrange
> so that at least it does not needlessly restart from nid 0 each time.
> And add a couple of comments: here is why we keep pfn instead of page.
> 
> Signed-off-by: Hugh Dickins <hughd@...gle.com>
> ---
>  mm/ksm.c |   38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> --- mmotm.orig/mm/ksm.c	2013-01-25 14:36:52.152205940 -0800
> +++ mmotm/mm/ksm.c	2013-01-25 14:36:53.244205966 -0800
> @@ -1830,31 +1830,36 @@ void ksm_migrate_page(struct page *newpa
>  #endif /* CONFIG_MIGRATION */
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -static struct stable_node *ksm_check_stable_tree(unsigned long start_pfn,
> -						 unsigned long end_pfn)
> +static void ksm_check_stable_tree(unsigned long start_pfn,
> +				  unsigned long end_pfn)
>  {
> +	struct stable_node *stable_node;
>  	struct rb_node *node;
>  	int nid;
>  
> -	for (nid = 0; nid < nr_node_ids; nid++)
> -		for (node = rb_first(&root_stable_tree[nid]); node;
> -				node = rb_next(node)) {
> -			struct stable_node *stable_node;
> -
> +	for (nid = 0; nid < nr_node_ids; nid++) {
> +		node = rb_first(&root_stable_tree[nid]);
> +		while (node) {

This is not your fault, the old code is wrong too. It is assuming that all
nodes are populated in numeric orders with no holes. It won't work if just
two nodes 0 and 4 are online. It should be using for_each_online_node().

-- 
Mel Gorman
SUSE Labs
--
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/