2017-04-04 18:41+0200, Andrew Jones: > On Tue, Apr 04, 2017 at 05:30:14PM +0200, Christoffer Dall wrote: >> On Fri, Mar 31, 2017 at 06:06:50PM +0200, Andrew Jones wrote: >> > From: Radim Krčmář <rkrcmar@xxxxxxxxxx> >> > >> > A first step in vcpu->requests encapsulation. >> >> Could we have a note here on why we need to access vcpu->requests using >> READ_ONCE now? > > Sure, maybe we should put the note as a comment above the read in > kvm_request_pending(). Something like > > /* > * vcpu->requests reads may appear in sequences that have strict > * data or control dependencies. Use READ_ONCE() to ensure the > * compiler does not do anything that breaks the required ordering. > */ > > Radim? Uses of vcpu->requests should already have barriers that take care of the ordering. I think the main reason for READ_ONCE() is to tell programmers that requests are special, but predictable. READ_ONCE() is not necessary in any use I'm aware of, but there is no harm in telling the compiler that vcpu->requests are what we think they are ... /* * vcpu->requests are a lockless synchronization mechanism, where * memory barriers are necessary for correct behavior, see * Documentation/virtual/kvm/vcpu-requests.rst. * * READ_ONCE() is not necessary for correctness, but simplifies * reasoning by constricting the generated code. */ I considered READ_ONCE() to be self-documenting. :)
