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

From: Paolo Bonzini <pbonzini@redhat.com>
To: Christoffer Dall <christoffer.dall@linaro.org>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>,
	"Christoffer Dall" <cdall@linaro.org>,
	"Andrew Jones" <drjones@redhat.com>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	"marc zyngier" <marc.zyngier@arm.com>
Subject: Re: [PATCH v2 1/9] KVM: add kvm_request_pending
Date: Sat, 8 Apr 2017 15:32:00 -0400 (EDT)	[thread overview]
Message-ID: <925034633.12169627.1491679920960.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20170408182314.GD29201@lvm>

> > 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?
> 
> >    *  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.

> Perhaps we should just let Drew respin at this point, in case he's
> confident about the right path, and then pick up from there?

I agree.

In any case, the memory barrier is the important part, but
adding READ_ONCE is self-documenting and I prefer to have it.

Paolo