Re: [PATCH] mhp: transfer dirty tag at radix_tree_replace

[Hirokazu Takahashi <taka@valinux.co.jp>]

Hi, Marcelo.

> > > 1) 
> > > I'm pretty sure you should transfer the radix tree tag at radix_tree_replace().
> > > If for example you transfer a dirty tagged page to another zone, an mpage_writepages()
> > > will miss it (because it uses pagevec_lookup_tag(PAGECACHE_DIRTY_TAG)). 
> > > 
> > > Should be quite trivial to do (save tags before deleting and set to new entry, 
> > > all in radix_tree_replace).
> > > 
> > > My implementation also contained the same bug.
> > 
> > Yes, it's one of the issues to do. The tag should be transferred in
> > radix_tree_replace() as you pointed out. The current implementation
> > sets the tag in set_page_dirty(newpage).
> 
> OK, guys, can you test this please?

Ok, I'll test it. 

> This transfer the dirty radix tree tag at radix_tree_replace, avoiding 
> a potential miss on tag-lookup.  We could also copy all bits representing 
> the valid tags for this node in the radix tree. 
> 
> But this uses the available interfaces from radix-lib.c. In case 
> a new tag gets added, radix_tree_replace() will have to know about it.

Yeah. I guess it would be better to copy the radix_tree_delete()
code to radix_tree_replace() and modify it to replace items directly
in the future.

> Pretty straightforward.
> 
> I still need to figure out how to use Iwamoto's patch to add/remove 
> zone's on the fly (for testing the migration process).
>
> diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/include/linux/radix-tree.h linux-2.6.9-rc2-mm4/include/linux/radix-tree.h
> --- linux-2.6.9-rc2-mm4.mhp.orig/include/linux/radix-tree.h	2004-10-05 15:09:39.198873072 -0300
> +++ linux-2.6.9-rc2-mm4/include/linux/radix-tree.h	2004-10-05 15:23:42.441680680 -0300
> @@ -68,9 +68,17 @@
>  radix_tree_replace(struct radix_tree_root *root,
>  				unsigned long index, void *item)
>  {
> +	int dirty;
> +
> +	dirty = radix_tree_tag_get(root, index, PAGECACHE_TAG_DIRTY);
> +
>  	if (radix_tree_delete(root, index) == NULL)
>  		return -1;
>  	radix_tree_insert(root, index, item);
> +
> +	if (dirty)
> +		radix_tree_tag_set(root, index, PAGECACHE_TAG_DIRTY);
> +
>  	return 0;
>  }
>  
> diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/lib/radix-tree.c linux-2.6.9-rc2-mm4/lib/radix-tree.c
> --- linux-2.6.9-rc2-mm4.mhp.orig/lib/radix-tree.c	2004-10-05 15:09:29.442356288 -0300
> +++ linux-2.6.9-rc2-mm4/lib/radix-tree.c	2004-10-05 15:24:16.961432880 -0300
> @@ -443,7 +443,6 @@
>  }
>  EXPORT_SYMBOL(radix_tree_tag_clear);
>  
> -#ifndef __KERNEL__	/* Only the test harness uses this at present */
>  /**
>   *	radix_tree_tag_get - get a tag on a radix tree node
>   *	@root:		radix tree root
> @@ -495,7 +494,6 @@
>  	}
>  }
>  EXPORT_SYMBOL(radix_tree_tag_get);
> -#endif
>  
>  static unsigned int
>  __lookup(struct radix_tree_root *root, void **results, unsigned long index,
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>
> 
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>