Re: KVM: guest: only batch user pte updates

[Jeremy Fitzhardinge <jeremy@goop.org>]

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