From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Fitzhardinge Subject: Re: KVM: guest: only batch user pte updates Date: Tue, 10 Feb 2009 15:14:48 -0800 Message-ID: <49920A68.5030408@goop.org> References: <20090210214532.GA4082@amt.cnet> <4991FD0D.1070108@goop.org> <20090210224141.GA4471@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Avi Kivity , kvm-devel , Rusty Russell , Zachary Amsden To: Marcelo Tosatti Return-path: Received: from gw.goop.org ([64.81.55.164]:46111 "EHLO mail.goop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756232AbZBJXOu (ORCPT ); Tue, 10 Feb 2009 18:14:50 -0500 In-Reply-To: <20090210224141.GA4471@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > 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 ? > Well, that depends on how expensive each update is. For something like kunmap atomic, I think combining the clear+tlb flush probably is worthwhile. > Until someone forgets about arch_flush_lazy_mmu_mode again... > It has been surprisingly unproblematic, though this CPA issue came to light. But given that there's only a few "correct" ways to update the kernel mappings now (kmap/vmap/vmalloc, kmap_atomic and cpa, I think), it should be easy to cover all the bases. (Hm, better check vmap.) J