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
