Re: [patch 3/5] KVM: add kvm_follow_page()

[Marcelo Tosatti <marcelo-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org>]

On Mon, Dec 24, 2007 at 08:56:01AM +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
> >It can acquire the pagetablelock in the callback handler. But then,
> >vm_normal_page() must also be exported.
> >
> >Are you guys OK with this ?
> >
> >  
> 
> Seems to me that requires fairly detailed mucking in mm details, just to 
> get at a page.

I agree.

> I believe that a new get_users_pages_inatomic() is more suitable; 
> Andrew, I'll write it if you agree.  Alternatively, walk_page_range() 
> should take the lock itself, otherwise it is only usable if you don't 
> care about correctness?

Why not just export follow_page() ? That is exactly what KVM needs.

> >+static int kvm_pte_range(pmd_t *pmd, unsigned long addr, unsigned long 
> >end,
> >+		           void *private)
> >+{
> >+	struct page **page = private;
> >+	struct vm_area_struct *vma;
> >+	pte_t *ptep, pte;
> >+	spinlock_t *ptl;
> >+	int err = -EFAULT;
> >+
> >+	vma = find_vma(current->mm, addr);
> >+	if (!vma)
> >+		return err;
> >+
> >+	ptep = pte_offset_map_lock(current->mm, pmd, addr, &ptl);
> >+	pte = *ptep;
> >+	if (!pte_present(pte))
> >+		goto unlock;
> >+
> >+	*page = vm_normal_page(vma, addr, pte);
> >+	if (!*page)
> >+		goto unlock;
> >+
> >+	get_page(*page);
> >+	err = 0;
> >+unlock:
> >+	pte_unmap_unlock(ptep, ptl);
> >+	return err;
> >+}
> >+
> >  
> 
> See what one has to write to get at a page?  Unreasonable IMO.  And 
> that's without hugetlb support AFAICT (kvm will want that soon).

Indeed... Its pretty much duplicating what follow_page() already does.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/