Re: [PATCH 15/24] kvm: mmu: Wrap mmu_lock cond_resched and needbreak

Ben Gardon
2021-01-27T13:19:10Z

On Tue, Jan 26, 2021 at 6:38 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 21/01/21 01:19, Sean Christopherson wrote:
> > What if we simply make the common mmu_lock a union? The rwlock_t is
> > probably a bit bigger, but that's a few bytes for an entire VM. And
> > maybe this would entice/inspire other architectures to move to a similar
> > MMU model.
>
> Looking more at this, there is a problem in that MMU notifier functions
> take the MMU lock.
>
> Yes, qrwlock the size is a bit larger than qspinlock. However, the fast
> path of qrwlocks is small, and if the slow paths are tiny compared to
> the mmu_lock critical sections that are so big as to require
> cond_resched. So I would consider just changing all architectures to an
> rwlock.

I like the idea of putting the MMU lock union directly in struct KVM
and will make that change in the next version of this series. In my
testing, I found that wholesale replacing the spin lock with an rwlock
had a noticeable negative performance impact on the legacy / shadow
MMU. Enough that it motivated me to implement this more complex union
scheme. While the difference was pronounced in the dirty log perf test
microbenchmark, it's an open question as to whether it would matter in
practice.

>
> Paolo
>