On Wed, Mar 31, 2021, Sean Christopherson wrote: > On Wed, Mar 31, 2021, Paolo Bonzini wrote: > > On 31/03/21 21:47, Sean Christopherson wrote: > > I also thought of busy waiting on down_read_trylock if the MMU notifier > > cannot block, but that would also be invalid for the opposite reason (the > > down_write task might be asleep, waiting for other readers to release the > > task, and the down_read_trylock busy loop might not let that task run). > > > > > And that's _already_ the worst case since notifications are currently > > > serialized by mmu_lock. > > > > But right now notifications are not a single critical section, they're two, > > aren't they? > > Ah, crud, yes. Holding a spinlock across the entire start() ... end() would be > bad, especially when the notifier can block since that opens up the possibility > of the task sleeping/blocking/yielding while the spinlock is held. Bummer. On a related topic, any preference on whether to have an explicit "must_lock" flag (what I posted), or derive the logic based on other params? The helper I posted does: if (range->must_lock && kvm_mmu_lock_and_check_handler(kvm, range, &locked)) goto out_unlock; but it could be: if (!IS_KVM_NULL_FN(range->on_lock) && !range->may_block && kvm_mmu_lock_and_check_handler(kvm, range, &locked)) goto out_unlock; The generated code should be nearly identical on a modern compiler, so it's purely a question of aesthetics. I slightly prefer the explicit "must_lock" to avoid spreading out the logic too much, but it also feels a bit superfluous.
