Re: [PATCH 4/11] ksm: reorganize ksm_check_stable_tree

[Mel Gorman]

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