Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <442859af-6454-b15e-b2ad-0fc7c4e22909@redhat.com>
Date:   Wed, 2 Mar 2022 21:14:25 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v3 22/28] KVM: x86/mmu: Zap defunct roots via asynchronous
 worker
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>
Cc:     Christian Borntraeger <borntraeger@linux.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        David Hildenbrand <david@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, David Matlack <dmatlack@google.com>,
        Ben Gardon <bgardon@google.com>,
        Mingwei Zhang <mizhang@google.com>
References: <20220226001546.360188-1-seanjc@google.com>
 <20220226001546.360188-23-seanjc@google.com>
 <b9270432-4ee8-be8e-8aa1-4b09992f82b8@redhat.com>
 <Yh+xA31FrfGoxXLB@google.com>
 <f4189f26-eff9-9fd0-40a1-69ac7759dedf@redhat.com>
 <Yh/GoUPxMRyFqFc5@google.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <Yh/GoUPxMRyFqFc5@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/2/22 20:33, Sean Christopherson wrote:
> What about that idea?  Put roots invalidated by "fast zap" on_another_  list?
> My very original idea of moving the roots to a separate list didn't work because
> the roots needed to be reachable by the mmu_notifier.  But we could just add
> another list_head (inside the unsync_child_bitmap union) and add the roots to
> _that_  list.

Perhaps the "separate list" idea could be extended to have a single 
worker for all kvm_tdp_mmu_put_root() work, and then indeed replace 
kvm_tdp_mmu_zap_invalidated_roots() with a flush of _that_ worker.  The 
disadvantage is a little less parallelism in zapping invalidated roots; 
but what is good for kvm_tdp_mmu_zap_invalidated_roots() is just as good 
for kvm_tdp_mmu_put_root(), I suppose.  If one wants separate work 
items, KVM could have its own workqueue, and then you flush that workqueue.

For now let's do it the simple but ugly way.  Keeping 
next_invalidated_root() does not make things worse than the status quo, 
and further work will be easier to review if it's kept separate from 
this already-complex work.

Paolo