Re: [kvm-devel] performance with guests running 2.4 kernels (specifically RHEL3)

[Avi Kivity <avi@qumranet.com>]

Andrea Arcangeli wrote:
>
>>> - set up kmap to point at pte
>>> - test_and_clear_bit(pte)
>>> - kunmap
>>>
>>> From kvm's point of view this looks like
>>>
>>> - several accesses to set up the kmap
>>>       
>
> Hmm, the kmap establishment takes a single guest operation in the
> fixmap area. That's a single write to the pte, to write a pte_t 8/4
> byte large region (PAE/non-PAE). The same pte_t is then cleared and
> flushed out of the tlb with a cpu-local invlpg during kunmap_atomic.
>
> I count 1 write here so far.
>
>   

No, two:

static inline void set_pte(pte_t *ptep, pte_t pte)
{
        ptep->pte_high = pte.pte_high;
        smp_wmb();
        ptep->pte_low = pte.pte_low;
}


>>>  - if these accesses trigger flooding, we will have to tear down the
>>> shadow for this page, only to set it up again soon
>>>       
>
> So the shadow mapping the fixmap area would be tear down by the
> flooding.
>   

Before we started patching this, yes.

> Or is the shadow corresponding to the real user pte pointed by the
> fixmap, that is unshadowed by the flooding, or both/all?
>
>   

After we started patching this, no, but with per-page-pte-history, yes 
(correctly).

>>> - an access to the pte (emulted)
>>>       
>
> Here I count the second write and this isn't done on the fixmap area
> like the first write above, but this is a write to the real user pte,
> pointed by the fixmap. So if this is emulated it means the shadow of
> the user pte pointing to the real data page is still active.
>   

Right.  But if we are scanning a page table linearly, it should be 
unshadowed.

>   
>>>  - if this access _doesn't_ trigger flooding, we will have 512 unneeded
>>> emulations.  The pte is worthless anyway since the accessed bit is clear
>>> (so we can't set up a shadow pte for it)
>>>    - this bug was fixed
>>>       
>
> You mean the accessed bit on fixmap pte used by kmap? Or the user pte
> pointed by the fixmap pte?
>   

The user pte.  After guest code runs test_and_clear_bit(accessed_bit, 
ptep), we can't shadow that pte (all shadowed ptes must have the 
accessed bit set in the corresponding guest pte, similar to how a tlb 
entry can only exist if the accessed bit is set).

>   
>>> - an access to tear down the kmap
>>>       
>
> Yep, pte_clear on the fixmap pte_t followed by an invlpg (if that
> matters).
>   

Looking at the code, that only happens if CONFIG_HIGHMEM_DEBUG is set.

> I think what we should aim for is to quickly reach this condition:
>
> 1) always keep the fixmap/kmap pte_t shadowed and emulate the
>    kmap/kunmap access so the test_and_clear_young done on the user pte
>    doesn't require to re-establish the spte representing the fixmap
>    virtual address. If we don't emulate fixmap we'll have to
>    re-establish the spte during the write to the user pte, and
>    tear it down again during kunmap_atomic. So there's not much doubt
>    fixmap access emulation is worth it.
>   

That is what is done by current HEAD.  
418c6952ba9fd379059ed325ea5a3efe904fb7fd is responsible.

Note that there is an alternative: allow the kmap pte to be unshadowed, 
and instead emulate the access through that pte (i.e. emulate the btc 
instruction).  I don't think it's worth it though because it hurts other 
users of the fixmap page.
> 2) get rid of the user pte shadow mapping pointing to the user data so
>    the test_and_clear of the young bitflag on the user pte will not be
>    emulated and it'll run at full CPU speed through the shadow pte
>    mapping corresponding to the fixmap virtual address
>   

That's what per-page-pte-history is supposed to do.  The first few 
accesses are emulated, the next will be native.

It's still not full speed as the kmap setup has to be emulated (twice).

One possible optimization is that if we see the first part of the kmap 
instantiation, we emulate a few more instructions before returning to 
the guest.  Xen does this IIRC.


> kscand pattern is the same as running mprotect on a 32bit 2.6
> kernel so it sounds worth optimizing for it, even if kscand may be
> unfixable without killall -STOP kscand or VM fixes to guest.
>
>   

I'm no longer sure the access pattern is sequential, since I see 
kmap_atomic() will not recreate the pte if its value has not changed 
(unless HIGHMEM_DEBUG).

-- 
error compiling committee.c: too many arguments to function