Re: [PATCH v2 1/9] KVM: add kvm_request_pending

["Radim Krčmář" <rkrcmar@redhat.com>]

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. :)