From: Andrea Arcangeli <andrea@qumranet.com>
To: Marcelo Tosatti <marcelo@kvack.org>
Cc: kvm-devel <kvm-devel@lists.sourceforge.net>,
Avi Kivity <avi@qumranet.com>
Subject: Re: KVM: MMU: add KVM_ZAP_GFN ioctl
Date: Wed, 26 Mar 2008 13:31:13 +0100 [thread overview]
Message-ID: <20080326123113.GB657@v2.random> (raw)
In-Reply-To: <20080324065427.GA8458@v2.random>
On Mon, Mar 24, 2008 at 07:54:27AM +0100, Andrea Arcangeli wrote:
> I'd more accurately describe the race as this:
>
>
> CPU0 CPU1
>
> spte = rmap_next(kvm, rmapp, NULL);
> while (spte) {
> BUG_ON(!spte);
> BUG_ON(!(*spte & PT_PRESENT_MASK));
> rmap_printk("rmap_nuke: spte %p %llx\n", spte, *spte);
> page_count = 2
> <---------- try_to_unmap_one()
> page_count = 1
> rmap_remove(kvm, spte); <- page is freed, race window opened
> set_shadow_pte(spte, shadow_trap_nonpresent_pte);
> nuked = 1;
> spte = rmap_next(kvm, rmapp, spte);
> }
> if (nuked)
> kvm_flush_remote_tlbs(kvm); <- race window closed
As I suspected, it seems this race can happen even when sptes are
removed from the cache. The VM may move the page_count to swapcount
while rmap_remove runs. Nobody has ever reproduced it, but it has to
be fixed nevertheless.
My mmu notifier patch closed this race condition against the linux VM,
by doing the last free with the right ordering under the PG_lock, in
turn ensuring when the VM removes the page from anonymous memory, the
tlb of the secondary mmu is flushed before the final put_page.
We discussed with Avi to do the right ordering all the time in
rmap_remove so there's no dependency on the Linux VM locking when
calling mmu notifier methods, and to be more robust in general (so
unmap all sptes, queue all the pages to unpin in a global array inside
the kvm_arch, flush the tlb once, put_page all the pages in the
array). As Avi pointed out we can't use page->lru for the queue.
Then perhaps your KVM ioctl will just work safe.
Marcelo, if you're interested to fix this let us know, so we don't
duplicate effort, or Avi or me can do the fix on rmap_remove.
Thanks!
Andrea
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
next prev parent reply other threads:[~2008-03-26 12:31 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-12 22:29 KVM: MMU: add KVM_ZAP_GFN ioctl Marcelo Tosatti
2008-03-20 12:09 ` Avi Kivity
2008-03-21 11:42 ` Andrea Arcangeli
2008-03-21 13:37 ` Marcelo Tosatti
2008-03-21 15:56 ` Andrea Arcangeli
2008-03-21 21:23 ` Marcelo Tosatti
2008-03-24 6:54 ` Andrea Arcangeli
2008-03-26 12:31 ` Andrea Arcangeli [this message]
2008-03-23 8:50 ` Avi Kivity
2008-03-23 20:23 ` Andrea Arcangeli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080326123113.GB657@v2.random \
--to=andrea@qumranet.com \
--cc=avi@qumranet.com \
--cc=kvm-devel@lists.sourceforge.net \
--cc=marcelo@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox