From: Marcelo Tosatti <mtosatti@redhat.com>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Avi Kivity <avi@redhat.com>, kvm-devel <kvm@vger.kernel.org>,
Rusty Russell <rusty@rustcorp.com.au>,
Zachary Amsden <zach@vmware.com>
Subject: Re: KVM: guest: only batch user pte updates
Date: Tue, 10 Feb 2009 20:41:41 -0200 [thread overview]
Message-ID: <20090210224141.GA4471@amt.cnet> (raw)
In-Reply-To: <4991FD0D.1070108@goop.org>
On Tue, Feb 10, 2009 at 02:17:49PM -0800, Jeremy Fitzhardinge wrote:
> Marcelo Tosatti wrote:
>> KVM's paravirt mmu pte batching has issues with, at least, kernel
>> updates from DEBUG_PAGEALLOC.
>>
>> This has been experienced with slab allocation from irq context from
>> within lazy mmu sections:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=480822
>>
>> DEBUG_PAGEALLOC will map/unmap the kernel pagetables to catch bad
>> accesses, with code such as:
>>
>> __change_page_attr():
>>
>> /*
>> * Do we really change anything ?
>> */
>> if (pte_val(old_pte) != pte_val(new_pte)) {
>> set_pte_atomic(kpte, new_pte);
>> cpa->flags |= CPA_FLUSHTLB;
>> }
>>
>> A present->nonpresent update can be queued, but not yet committed to
>> memory. So the set_pte_atomic will be skipped but the update flushed
>> afterwards. set_pte_ATOMIC.
>>
>
> Are you saying that there's a queued update which means that old_pte is
> a stale value which happens to equal new_pte, so new_pte is never set?
> OK, sounds like a generic problem, of the same sort we've had with
> kmap_atomic being used in interrupt routines in lazy mode.
Yes. It seems however that only set_pte_at/pte_update/_defer are
used under significatly long lazy mmu sections (long as in number of
updates). Is it worthwhile to bother (and risk) batching kernel pte updates ?
Until someone forgets about arch_flush_lazy_mmu_mode again...
> In this case, I think the proper fix is to call
> arch_flush_lazy_mmu_mode() before reading old_pte to make sure its up to
> date, and calling it again when processing CPA_FLUSHTLB.
> Could you try the patch below instead?
It should work yes.
> (BTW, set_pte_atomic doesn't mean synchronous; it just means its safe to
> use on live ptes on 32-bit PAE machines which can't otherwise atomically
> update a pte.)
Doh, of course.
next prev parent reply other threads:[~2009-02-10 22:42 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-10 21:45 KVM: guest: only batch user pte updates Marcelo Tosatti
2009-02-10 22:17 ` Jeremy Fitzhardinge
2009-02-10 22:41 ` Marcelo Tosatti [this message]
2009-02-10 23:14 ` Jeremy Fitzhardinge
2009-02-11 11:56 ` Avi Kivity
2009-02-11 16:57 ` Jeremy Fitzhardinge
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=20090210224141.GA4471@amt.cnet \
--to=mtosatti@redhat.com \
--cc=avi@redhat.com \
--cc=jeremy@goop.org \
--cc=kvm@vger.kernel.org \
--cc=rusty@rustcorp.com.au \
--cc=zach@vmware.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.