Re: [PATCH] rmap: Fix Bugzilla Bug #5493

[Johannes Weiner <hannes@cmpxchg.org>]

Hi Christian,

On Sat, Mar 06, 2010 at 02:02:12AM +0100, Christian Ehrhardt wrote:
> diff --git a/arch/arm/mm/fault-armv.c b/arch/arm/mm/fault-armv.c
> index c9b97e9..4b8d01f 100644
> --- a/arch/arm/mm/fault-armv.c
> +++ b/arch/arm/mm/fault-armv.c
> @@ -117,7 +117,8 @@ make_coherent(struct address_space *mapping, struct vm_area_struct *vma,
>  	 * cache coherency.
>  	 */
>  	flush_dcache_mmap_lock(mapping);
> -	vma_prio_tree_foreach(mpnt, &iter, &mapping->i_mmap, pgoff, pgoff) {
> +	vma_prio_tree_foreach(mpnt, struct vm_area_struct, shared, &iter,
> +				&mapping->i_mmap, pgoff, pgoff) {

How about vma_file_tree_foreach() vs. vma_anon_tree_foreach()?  I found that
to be more descriptive (and it fits the users into a single line again ;).

>  #define INIT_PRIO_TREE_ROOT(ptr)	__INIT_PRIO_TREE_ROOT(ptr, 0)
> -#define INIT_RAW_PRIO_TREE_ROOT(ptr)	__INIT_PRIO_TREE_ROOT(ptr, 1)
> +#define INIT_SHARED_PRIO_TREE_ROOT(ptr)	__INIT_PRIO_TREE_ROOT(ptr, 1)
> +#define INIT_ANON_PRIO_TREE_ROOT(ptr)	__INIT_PRIO_TREE_ROOT(ptr, 2)

SHARED vs. PRIVATE?

> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -207,7 +207,7 @@ static void __remove_shared_vm_struct(struct vm_area_struct *vma,
>  	if (unlikely(vma->vm_flags & VM_NONLINEAR))
>  		list_del_init(&vma->shared.vm_set.list);
>  	else
> -		vma_prio_tree_remove(vma, &mapping->i_mmap);
> +		vma_prio_tree_remove(&vma->shared, &mapping->i_mmap);
>  	flush_dcache_mmap_unlock(mapping);
>  }
>  
> @@ -430,7 +430,7 @@ static void __vma_link_file(struct vm_area_struct *vma)
>  		if (unlikely(vma->vm_flags & VM_NONLINEAR))
>  			vma_nonlinear_insert(vma, &mapping->i_mmap_nonlinear);
>  		else
> -			vma_prio_tree_insert(vma, &mapping->i_mmap);
> +			vma_prio_tree_insert(&vma->shared, &mapping->i_mmap);
>  		flush_dcache_mmap_unlock(mapping);
>  	}
>  }
> @@ -593,9 +593,9 @@ again:			remove_next = 1 + (end > next->vm_end);
>  
>  	if (root) {
>  		flush_dcache_mmap_lock(mapping);
> -		vma_prio_tree_remove(vma, root);
> +		vma_prio_tree_remove(&vma->shared, root);
>  		if (adjust_next)
> -			vma_prio_tree_remove(next, root);
> +			vma_prio_tree_remove(&next->shared, root);
>  	}
>  
>  	vma->vm_start = start;
> @@ -608,8 +608,8 @@ again:			remove_next = 1 + (end > next->vm_end);
>  
>  	if (root) {
>  		if (adjust_next)
> -			vma_prio_tree_insert(next, root);
> -		vma_prio_tree_insert(vma, root);
> +			vma_prio_tree_insert(&next->shared, root);
> +		vma_prio_tree_insert(&vma->shared, root);
>  		flush_dcache_mmap_unlock(mapping);
>  	}

What's with expand_stack()?  This changes the radix index or the heap
index, depending on the direction in which the stack grows, but it
does not adjust the tree and so its order is violated.  Did you make
sure that this is fine?
  
	Hannes