2016-02-18 19:08+0100, Paolo Bonzini: > On 17/02/2016 20:14, Radim Krčmář wrote: >> + /* pit->pit_state.lock was overloaded to prevent userspace from getting >> + * an inconsistent state after running multiple KVM_REINJECT_CONTROL >> + * ioctls in parallel. Use a separate lock if that ioctl isn't rare. >> + */ >> + mutex_lock(&pit->pit_state.lock); >> + kvm_pit_set_reinject(pit, control->pit_reinject); >> + mutex_unlock(&pit->pit_state.lock); > > ... so in patch 7 concurrent _writes_ of reinject are protected by the > lock, but reads are done outside it (in pit_timer_fn). WDYT about > making reinject an atomic_t?
There was/is no harm in having reinject non-atomic. This patch added notifiers, which is the reason for re-introducing a mutex. Userspace can (and SHOULDN'T) call this function multiple times, concurrently, so the mutex prevents a situations where, e.g. only one notifier is registered in the end. I thought about really stupid stuff when doing this series ...
