Re: [PATCH 2.6.21-rc3-mm2 3/4] futex_requeue_pi optimization

[Peter Zijlstra <a.p.zijlstra@chello.nl>]

On Tue, 2007-03-20 at 16:32 +0100, Pierre Peiffer wrote:
> Peter Zijlstra a écrit :
> >> +static void *get_futex_address(union futex_key *key)
> >> +{
> >> +	void *uaddr;
> >> +
> >> +	if (key->both.offset & 1) {
> >> +		/* shared mapping */
> >> +		uaddr = (void*)((key->shared.pgoff << PAGE_SHIFT)
> >> +				+ key->shared.offset - 1);
> >> +	} else {
> >> +		/* private mapping */
> >> +		uaddr = (void*)(key->private.address + key->private.offset);
> >> +	}
> >> +
> >> +	return uaddr;
> >> +}
> > 
> > This will not work for nonlinear vmas, granted, not a lot of ppl stick
> > futexes in nonlinear vmas, but the futex_key stuff handles it, this
> > doesn't.
> 
> Indeed ! Thanks for pointing me to this.
> 
> Since I'm not familiar with vmm, does this code look more correct to you ?

Unfortunately not, nonlinear vmas don't have a linear relation between
address and offset. What you would need to do is do a linear walk of the
page tables. But even that might not suffice if nonlinear vmas may form
a non-injective, surjective mapping.

/me checks..

Hmm, yes that seems valid, so in general, this reverse mapping does not
uniquely exist for non-linear vmas. :-(

What to do... disallow futexes in nonlinear mappings, store the address
in the key?

> static void *get_futex_address(union futex_key *key)
> {
> 	void *uaddr;
> 	struct vm_area_struct *vma = current->mm->mmap;
> 
> 	if (key->both.offset & 1) {
> 		/* shared mapping */
> 		struct file * vmf;
> 
> 		do {
> 			if ((vmf = vma->vm_file)
> 			    && (key->shared.inode == vmf->f_dentry->d_inode))
> 				break;
> 			vma = vma->vm_next;
> 		} while (vma);
> 
> 		if (likely(!(vma->vm_flags & VM_NONLINEAR)))
> 			uaddr = (void*)((key->shared.pgoff << PAGE_SHIFT)
> 					+ key->shared.offset - 1);
> 		else
> 			uaddr = (void*) vma->vm_start
> 				+ ((key->shared.pgoff - vma->vm_pgoff)
> 				   << PAGE_SHIFT)
> 				+ key->shared.offset - 1;
> 	} else {
> 		/* private mapping */
> 		uaddr = (void*)(key->private.address + key->private.offset);
> 	}
> 
> 	return uaddr;
> }
> 
> Or is there a more direct way to retrieve the vma corresponding to the given inode ?

the vma_prio_tree would be able to give all vmas associated with a
mapping.