Re: [PATCH 1/3] KVM: s390: clear kicked_mask before sleeping again

[Halil Pasic]

On Wed, 20 Oct 2021 08:08:16 +0200
Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx> wrote:

> > >> +	clear_bit(vcpu->vcpu_idx, vcpu->kvm->arch.gisa_int.kicked_mask);    
> > > 
> > > so, you unconditionally clear the flag, before knowing if the vCPU is
> > > runnable?

Right. I talked about this with  Mimu. It would extend the section
guarded by the bit, and than may be a good thing. Maybe we should
measure that alternative as well.

> > > 
> > > from your description I would have expected to only clear the bit if
> > > the vCPU is not runnable.
> > > 
> > > would things break if we were to try to kick the vCPU again after
> > > clearing the bit, but before dispatching it?    
> > 
> > The whole logic is just an optimization to avoid unnecessary wakeups.
> > When the bit is set a wakup might be omitted.
> > I prefer to do an unneeded wakeup over not doing a wakeup so I think
> > over-clearing is safer.
> > In fact, getting rid of this micro-optimization would be a valid
> > alternative.  
> 
> my only concern was if things would break in case we kick the vCPU
> again after clearing the bit; it seems nothing breaks, so I'm ok with it

I'm not sure about the exact impact of over-waking.
kvm_s390_vcpu_wakeup() sets vcpu->valid_wakeup which is I believe used
for some halt poll heuristics. We unset that in
kvm_arch_vcpu_block_finish(). If we cleared only conditionally the
protection would extend for that as well. Which would be a good thing.
The statistics stuff in kvm_vcpu_wake_up() does account for already
running, so I see no correctness issues there.

Regards,
Halil