From patchwork Tue Feb 10 22:17:49 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Fitzhardinge X-Patchwork-Id: 6531 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n1AMIBhS012138 for ; Tue, 10 Feb 2009 22:18:12 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755509AbZBJWRx (ORCPT ); Tue, 10 Feb 2009 17:17:53 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756005AbZBJWRx (ORCPT ); Tue, 10 Feb 2009 17:17:53 -0500 Received: from gw.goop.org ([64.81.55.164]:56473 "EHLO mail.goop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755509AbZBJWRw (ORCPT ); Tue, 10 Feb 2009 17:17:52 -0500 Received: by lurch.goop.org (Postfix, from userid 525) id 960242C8058; Tue, 10 Feb 2009 14:17:51 -0800 (PST) Received: from lurch.goop.org (localhost [127.0.0.1]) by lurch.goop.org (Postfix) with ESMTP id 2265B2C8045; Tue, 10 Feb 2009 14:17:51 -0800 (PST) Received: from abulafia.goop.org (adsl-69-107-80-209.dsl.pltn13.pacbell.net [69.107.80.209]) by lurch.goop.org (Postfix) with ESMTPSA; Tue, 10 Feb 2009 14:17:51 -0800 (PST) Message-ID: <4991FD0D.1070108@goop.org> Date: Tue, 10 Feb 2009 14:17:49 -0800 From: Jeremy Fitzhardinge User-Agent: Thunderbird 2.0.0.19 (X11/20090105) MIME-Version: 1.0 To: Marcelo Tosatti CC: Avi Kivity , kvm-devel , Rusty Russell , Zachary Amsden Subject: Re: KVM: guest: only batch user pte updates References: <20090210214532.GA4082@amt.cnet> In-Reply-To: <20090210214532.GA4082@amt.cnet> X-Enigmail-Version: 0.95.6 X-Virus-Scanned: ClamAV using ClamSMTP by lurch.goop.org Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org 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. 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? (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.) J commit 264d7d09de69b1f729adb43acc86bd504dd21251 Author: Jeremy Fitzhardinge Date: Tue Feb 10 14:15:52 2009 -0800 x86/cpa: make sure cpa is safe to call in lazy mmu mode The CPA code may be called while we're in lazy mmu update mode - for example, when using DEBUG_PAGE_ALLOC and doing a slab allocation in an interrupt handler which interrupted a lazy mmu update. In this case, the in memory pagetable state may be out of date due to pending queued updates. We need to flush any pending updates before inspecting the page table. Similarly, we must explicitly flush any modifications CPA may have made (which comes down to flushing queued operations when flushing the TLB). Signed-off-by: Jeremy Fitzhardinge --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c index 84ba748..fb12f06 100644 --- a/arch/x86/mm/pageattr.c +++ b/arch/x86/mm/pageattr.c @@ -576,6 +576,13 @@ static int __change_page_attr(struct cpa_data *cpa, int primary) else address = *cpa->vaddr; + /* + * If we're called with lazy mmu updates enabled, the + * in-memory pte state may be stale. Flush pending updates to + * bring them up to date. + */ + arch_flush_lazy_mmu_mode(); + repeat: kpte = lookup_address(address, &level); if (!kpte) @@ -854,6 +861,13 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages, } else cpa_flush_all(cache); + /* + * If we've been called with lazy mmu updates enabled, then + * make sure that everything gets flushed out before we + * return. + */ + arch_flush_lazy_mmu_mode(); + out: return ret; }