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

Marcelo Tosatti
2007-12-24T14:26:24Z

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.