On 03/09/2018 10:18 AM, Cornelia Huck wrote: > On Thu, 8 Mar 2018 13:30:09 +0000 > Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote: > >> For testing the exitless interrupt support it turned out useful to >> have separate counters for inject and delivery of I/O interrupt. >> While at it do the same for all interrupt types. For timer >> related interrupts (clock comparator and cpu timer) we even had >> no delivery counters. Fix this as well. On this way some counters >> are being renamed to have a similar name. > > Seems reasonable if you rely on the counters to have them cover > everything. Some meta-questions, though: > > - A lot of the places are also covered by trace events and/or dbf > events. These can add more information (like payload etc.), but are > not as lightweight as the counters. Does it also make sense to fill > in any gaps in that infrastructure? Or aren't they used as much? (I > haven't done much kvm debugging recently, so I'm probably not the > person to judge their usefulness.) In the end you will have a customer or QA issue that will require exactly that missing event to understand whats going on (Thats why I add these counters now). So yes, we might want to add more traces and/or dbf statements. This needs more work though, as both are more costly (e.g. dbf takes a spinlock) and/or need explicit enablement or the correct level. I will have a look. > > (more below...) > >> >> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx> >> --- >> arch/s390/include/asm/kvm_host.h | 25 +++++++++++++++++++++---- >> arch/s390/kvm/interrupt.c | 25 +++++++++++++++++++++---- >> arch/s390/kvm/kvm-s390.c | 24 +++++++++++++++++++++--- >> 3 files changed, 63 insertions(+), 11 deletions(-) > > (...) > >> @@ -1328,6 +1332,7 @@ static int __inject_extcall(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq) >> struct kvm_s390_extcall_info *extcall = &li->irq.extcall; >> uint16_t src_id = irq->u.extcall.code; >> >> + vcpu->stat.inject_external_call++; >> VCPU_EVENT(vcpu, 4, "inject: external call source-cpu:%u", >> src_id); >> trace_kvm_s390_inject_vcpu(vcpu->vcpu_id, KVM_S390_INT_EXTERNAL_CALL, > > Some of the injection functions (like this one) can return an error. > There does not seem to be any tracing of those failures, though. Would > it be helpful for at least some cases, or is that overkill? Hmm, that could be interesting, but it requires additional review to identify the proper places. I will see if I can come up with a followup patch later. > > Patch looks good to me, so > > Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx> > Thanks.
