Re: [PATCH v2 2/5] KVM: add KVM request variants without barrier

[Peter Xu]

On Wed, Mar 01, 2017 at 06:02:49PM +0100, Radim KrÄmÃÅ wrote:
> 2017-02-28 15:40+0800, Peter Xu:
> > On Tue, Feb 28, 2017 at 03:34:24PM +0800, Peter Xu wrote:
> > 
> > [...]
> > 
> >> > diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
> >> > index ee4af898bcf6..552ae2b5e911 100644
> >> > --- a/arch/mips/kvm/emulate.c
> >> > +++ b/arch/mips/kvm/emulate.c
> >> > @@ -865,7 +865,7 @@ enum emulation_result kvm_mips_emul_wait(struct kvm_vcpu *vcpu)
> >> >  		 * check if any I/O interrupts are pending.
> >> >  		 */
> >> >  		if (kvm_request_test_and_clear(KVM_REQ_UNHALT, vcpu)) {
> >> > -			clear_bit(KVM_REQ_UNHALT, &vcpu->requests);
> >> > +			__kvm_request_clear(KVM_REQ_UNHALT, vcpu);
> >> 
> >> Shall we just remove above line since we cleared it already?
> > 
> > Please ignore this since I see patch 4. :-)
> > 
> > It'll be nice if patch 4 will be before this one, but it's trivial.
> 
> I put [4/5] there to demonstrate that this error would have been less
> likely with the new naming.  I didn't expect that reviewers would go
> through the coccinelle transformation. :)

Yeah, I noticed it mostly because it's the first one touched.

Meanwhile, I think it's still worthwhile to go through the patch even
it's from cocinelle since sometimes coccinelle might do something that
we (or only me?) didn't expect. E.g., afaik it cannot handle well with
over-80-chars lines, so we need to wrap them on our own (I got a patch
from the author though to fix this, but not yet tested). And also
since patches are going to be merged changes, it just feel unsafe if
we merge something without reading it. :)

Thanks,

-- peterx