On Wed, Jan 17, 2024, David Woodhouse wrote: > On Wed, 2024-01-17 at 08:09 -0800, Sean Christopherson wrote: > > On Fri, Jan 12, 2024, David Woodhouse wrote: > > As you note above, some other mutex _should_ be held. I think we should lean > > into that. E.g. > > I don't. I'd like this code to stand alone *without* making the caller > depend on "some other lock" just for its own internal consistency. Hmm, I get where you're coming from, but protecting a per-vCPU asset with vcpu->mutex is completely sane/reasonable. Xen's refresh from a completely different task is the oddball. And unnecessarily taking multiple mutexes muddies the water, e.g. it's not clear what role kvm->arch.xen.xen_lock plays when it's acquired by kvm_xen_set_evtchn(). > > 1. Pass in the guarding mutex to kvm_gpc_init() and assert that said mutex is > > held for __refresh(), activate(), and deactivate(). > > 2. Fix the cases where that doesn't hold true. > > 3. Drop refresh_mutex > > > > I'll go for (3) but I disagree about (1) and (2). Just let the rwlock > work as $DEITY intended, which is what this patch is doing. It's a > cleanup. > > (And I didn't drop refresh_lock so far partly because it wants to be > done in a separate commit, but also because it does provide an > optimisation, as noted.
