On Tue, Jan 26, 2021 at 6:38 AM Paolo Bonzini [off-list ref] wrote:

On 21/01/21 01:19, Sean Christopherson wrote:

quoted

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