Messages in this thread | | | From | Paolo Bonzini <> | Subject | Re: [PATCH v2] kvm/selftests: Fix race condition with dirty_log_test | Date | Sat, 17 Apr 2021 14:59:48 +0200 |
| |
On 13/04/21 23:36, Peter Xu wrote: > This patch closes this race by allowing the main thread to give the vcpu thread > chance to do a VMENTER to complete that write operation. It's done by adding a > vcpu loop counter (must be defined as volatile as main thread will do read > loop), then the main thread can guarantee the vcpu got at least another VMENTER > by making sure the guest_vcpu_loops increases by 2. > > Dirty ring does not need this since dirty_ring_last_page would already help > avoid this specific race condition.
Just a nit, the comment and commit message should mention KVM_RUN rather than vmentry; it's possible to be preempted many times in vcpu_enter_guest without making progress, but those wouldn't return to userspace and thus would not update guest_vcpu_loops.
Also, volatile is considered harmful even in userspace/test code[1]. Technically rather than volatile one should use an atomic load (even a relaxed one), but in practice it's okay to use volatile too *for this specific use* (READ_ONCE/WRITE_ONCE are volatile reads and writes as well). If the selftests gained 32-bit support, one should not use volatile because neither reads or writes to uint64_t variables would be guaranteed to be atomic.
Queued, thanks.
Paolo
[1] Documentation/process/volatile-considered-harmful.rst
| |