lkml.org 
[lkml]   [2013]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v6 3/7] KVM: MMU: fast invalidate all pages
    On Wed, May 22, 2013 at 09:34:13AM +0300, Gleb Natapov wrote:
    > On Tue, May 21, 2013 at 10:33:30PM -0300, Marcelo Tosatti wrote:
    > > On Tue, May 21, 2013 at 11:39:03AM +0300, Gleb Natapov wrote:
    > > > > Any pages with stale information will be zapped by kvm_mmu_zap_all().
    > > > > When that happens, page faults will take place which will automatically
    > > > > use the new generation number.
    > > > >
    > > > > So still not clear why is this necessary.
    > > > >
    > > > This is not, strictly speaking, necessary, but it is the sane thing to do.
    > > > You cannot update page's generation number to prevent it from been
    > > > destroyed since after kvm_mmu_zap_all() completes stale ptes in the
    > > > shadow page may point to now deleted memslot. So why build shadow page
    > > > table with a page that is in a process of been destroyed?
    > >
    > > OK, can this be introduced separately, in a later patch, with separate
    > > justification, then?
    > >
    > > Xiao please have the first patches of the patchset focus on the problem
    > > at hand: fix long mmu_lock hold times.
    > >
    > > > Not sure what you mean again. We flush TLB once before entering this function.
    > > > kvm_reload_remote_mmus() does this for us, no?
    > >
    > > kvm_reload_remote_mmus() is used as an optimization, its separate from the
    > > problem solution.
    > >
    > > > >
    > > > > What was suggested was... go to phrase which starts with "The only purpose
    > > > > of the generation number should be to".
    > > > >
    > > > > The comment quoted here does not match that description.
    > > > >
    > > > The comment describes what code does and in this it is correct.
    > > >
    > > > You propose to not reload roots right away and do it only when root sp
    > > > is encountered, right? So my question is what's the point? There are,
    > > > obviously, root sps with invalid generation number at this point, so
    > > > reload will happen regardless in kvm_mmu_prepare_zap_page(). So why not
    > > > do it here right away and avoid it in kvm_mmu_prepare_zap_page() for
    > > > invalid and obsolete sps as I proposed in one of my email?
    > >
    > > Sure. But Xiao please introduce that TLB collapsing optimization as a
    > > later patch, so we can reason about it in a more organized fashion.
    >
    > So, if I understand correctly, you are asking to move is_obsolete_sp()
    > check from kvm_mmu_get_page() and kvm_reload_remote_mmus() from
    > kvm_mmu_invalidate_all_pages() to a separate patch.

    - Move the is_obsolete_sp() check from kvm_mmu_get_page() to a separate
    optimization patch.
    - Move kvm_reload_remote_mmus() optimization to a separate optimization
    patch.
    - Leave the is_obsolete_sp() check in kvm_mmu_invalidate_all_pages(), as
    it is necessary to guarantee forward progress, in the initial patches.

    > Fine by me, but if
    > we drop kvm_reload_remote_mmus() from kvm_mmu_invalidate_all_pages() the
    > call to kvm_mmu_invalidate_all_pages() in emulator_fix_hypercall() will
    > become nop. But I question the need to zap all shadow pages tables there
    > in the first place, why kvm_flush_remote_tlbs() is not enough?

    Good question.



    \
     
     \ /
      Last update: 2013-05-22 18:01    [W:4.743 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site