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

Andrea Arcangeli
2008-05-30T13:12:40Z

On Thu, May 29, 2008 at 06:16:55PM +0300, Avi Kivity wrote:
> Yes. We need a fault in order to set the guest accessed bit.

So what I'm missing now is how the spte corresponding to the user pte
that is under test_and_clear to clear the accessed bit, will not the
zapped immediately. If we don't zap it immediately, how do we set the
accessed bit again on the user pte, when the user program returned
running and used that shadow pte to access the program data after the
kscand pass?

Or am I missing something?

> Unshadowing a page is expensive, both in immediate cost, and in future cost
> of reshadowing the page and taking faults. It's worthwhile to be sure the
> guest really doesn't want it as a page table.

Ok that makes sense, but can we defer the unshadowing while still
emulating the accessed bit correctly on the user pte?

> If the pages are not scanned linearly, then unshadowing may not help.

It should help the second time kscand runs, for the user ptes that
aren't shadowed anymore, the second pass won't require any emulation
for test_and_bit because the spte of the fixmap area will be
read-write. The bug that passes the anonymous pages number instead of
the cache number will lead to many more test_and_clear than needed,
and not all user ptes may be used in between two different kscand passes.

> Let's see 1G of highmem is 250,000 pages, mapped by 500 pages tables.

There are likely 1500 ptes in highmem. (ram isn't the most important factor)

> Well, then after 4000 scans we ought to have unshadowed everything. So I
> guess per-page-pte-history is broken, can't explain it otherwise.

Yes, we should have unshadowed all user ptes after 4000 scans and then
the test_and_clear shouldn't require any more emulation, there will be
only 3 emulations for each kmap_atomic/kunmap_atomic.

> static inline void *__kmap_atomic(struct page *page, enum km_type type)
> {
> enum fixed_addresses idx;
> unsigned long vaddr;
>
> idx = type + KM_TYPE_NR*smp_processor_id();
> vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
> #if HIGHMEM_DEBUG
> if (!pte_none(*(kmap_pte-idx)))
> out_of_line_bug();
> #else
> /*
> * Performance optimization - do not flush if the new
> * pte is the same as the old one:
> */
> if (pte_val(*(kmap_pte-idx)) == pte_val(mk_pte(page, kmap_prot)))
> return (void *) vaddr;
> #endif
> set_pte(kmap_pte-idx, mk_pte(page, kmap_prot));
> __flush_tlb_one(vaddr);
>
> return (void*) vaddr;
> }

It's weird they optimized this if they enabled
CONFIG_HIGHMEM_DEBUG. Anyway it doesn't make a whole lot of difference
as it's an unlikely condition.

> (linux-2.4.21-47.EL)

Downloaded it now.

I think it should be clear that by now, we're trying to be
bug-compatile like the host here, and optimizing for 2.6 kmaps.