2017-04-08 15:32-0400, Paolo Bonzini:
> > > Makes sense. My pitch at the documentation after dropping READ_ONCE():
> >
> > I'm confused again, I thought you wanted to keep READ_ONCE().
> >
> > >
> > > /*
> > > * The return value of kvm_request_pending() is implicitly volatile
> >
> > why is that, actually?
"that" is the return value?
kvm_request_pending() is 'inline static' so the compiler can prove that
the function only returns vcpu->requests and that the surrounding code
doesn't change it, so various optimizations are possible.
Or the implicitly volatile bit?
We know that vcpu->requests can change at any time without action of the
execution thread, which makes it volatile, but we don't tell that to the
compiler, hence implicit.
READ_ONCE(vcpu->requests) is
*(volatile unsigned long *)&vcpu->requests
and makes it explicitly volatile.
> > > * and must be protected from reordering by the caller.
> > > */
> >
> > Can we be specific about what that means? (e.g. must be preceded by a
> > full smp_mb() - or whatever the case is).
>
> You can play devil's advocate both ways and argue that READ_ONCE is
> better, or that it is unnecessary hence worse. You can write good
> comments in either case. That's how I read Radim's message. But
> I think we all agree on keeping it in the end.
Exactly, I am in favor of READ_ONCE() as I don't like to keep the
compiler informed, just wanted to cover all options, because they are
not that different ... minute details are the hardest to decide. :)
