Andrea Arcangeli wrote:
> On Tue, Jan 22, 2008 at 04:08:16PM +0200, Avi Kivity wrote:
>
>> Andrea Arcangeli wrote:
>>
>>> This is the same as before but it uses the age_page callback to
>>> prevent the guest OS working set to be swapped out. It works well here
>>> so far. This depends on the memslot locking with mmu lock patch and on
>>> the mmu notifiers #v3 patch that I'll post in CC with linux-mm shortly
>>> that implements the age_page callback and that changes follow_page to
>>> set the young bit in the pte instead of setting the referenced bit (so
>>> the age_page will be called again later when the VM clears the young
>>> bit).
>>>
>>> +static void unmap_spte(struct kvm *kvm, u64 *spte)
>>> +{
>>> + struct page *page = pfn_to_page((*spte & PT64_BASE_ADDR_MASK) >>
>>> PAGE_SHIFT);
>>> + get_page(page);
>>> + rmap_remove(kvm, spte);
>>> + set_shadow_pte(spte, shadow_trap_nonpresent_pte);
>>> + kvm_flush_remote_tlbs(kvm);
>>> + __free_page(page);
>>> +}
>>>
>>>
>> Why is get_page()/__free_page() needed here? Isn't kvm_release_page_*()
>> sufficient?
>>
>
> The other-cpus-tlb have to be flushed _before_ the page is visible in
> the host kernel freelist, otherwise other host-cpus with tlbs still
> mapping the page with write-access would be able to modify the page
> even after it's queued in the freelist.
Right. But doesn't this apply to other callers of rmap_remove()?
Perhaps we need to put the flush in set_spte() or rmap_remove() and
rmap_write_protect().
Oh, rmap_write_protect() already has the flush.
> The mmu_notifier are called in
> places like munmap where the __free_page will not be a put_page but a
> real __free_page. Furthermore kvm_release_page_ aren't calling
> __free_page but put_page that would leak ram in those paths (mostly
> invalidate_range). I'd rather not depend on the mmu_notifiers always
> being invoked with an additional reference count on the page (in
> addition to the spte reference count). The ->invalidate_* methods
> might be the ones that put the page in the freelist.
>
I'm afraid I don't really understand the difference in semantics between
put_page() and __free_page(). Maybe we need to switch
kvm_release_page_*() to __free_page()?
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
