Re: [PATCH 1/2] KVM: s390: add vcpu stat counters for many instruction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 01/24/2018 03:45 PM, Cornelia Huck wrote:
> On Wed, 24 Jan 2018 12:32:34 +0100
> Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
> 
>> The overall instruction counter is larger than the sum of the
>> single counters. We should try to catch all instruction handlers
>> to make this match the summary counter.
>> Let us add sck,tb,sske,iske,rrbe,tb,tpi,tsch,lpsw,pswe.....
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> ---
>>  arch/s390/include/asm/kvm_host.h | 18 ++++++++++++++++--
>>  arch/s390/kvm/kvm-s390.c         | 18 ++++++++++++++++--
>>  arch/s390/kvm/priv.c             | 33 +++++++++++++++++++++++++++++++--
>>  3 files changed, 63 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index 2aee050..5d47991 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -1,7 +1,7 @@
>>  /*
>>   * definition for kernel virtual machines on s390
>>   *
>> - * Copyright IBM Corp. 2008, 2009
>> + * Copyright IBM Corp. 2008, 2018
>>   *
>>   * This program is free software; you can redistribute it and/or modify
>>   * it under the terms of the GNU General Public License (version 2 only)
>> @@ -323,18 +323,32 @@ struct kvm_vcpu_stat {
>>  	u64 deliver_program_int;
>>  	u64 deliver_io_int;
>>  	u64 exit_wait_state;
>> +	u64 instruction_epsw;
>> +	u64 instruction_gs;
>> +	u64 instruction_io;
> 
> I find the naming a bit confusing (tpi/tsch are I/O instructions, too)
> -- call this instruction_io_other?

yes will rename.


> 
>> +	u64 instruction_lpsw;
>> +	u64 instruction_lpswe;
>>  	u64 instruction_pfmf;
>> +	u64 instruction_ptff;
>> +	u64 instruction_sck;
>> +	u64 instruction_sckpf;
>>  	u64 instruction_stidp;
>>  	u64 instruction_spx;
>>  	u64 instruction_stpx;
>>  	u64 instruction_stap;
>> -	u64 instruction_storage_key;
>> +	u64 instruction_iske;
>> +	u64 instruction_ri;
>> +	u64 instruction_rrbe;
>> +	u64 instruction_sske;
>>  	u64 instruction_ipte_interlock;
>>  	u64 instruction_stsch;
>>  	u64 instruction_chsc;
> 
> The stsch and chsc counters are dead (probably have been for quite some
> time).

will remove
> 
>>  	u64 instruction_stsi;
>>  	u64 instruction_stfl;
>> +	u64 instruction_tb;
>> +	u64 instruction_tpi;
>>  	u64 instruction_tprot;
>> +	u64 instruction_tsch;
>>  	u64 instruction_sie;
>>  	u64 instruction_essa;
>>  	u64 instruction_sthyi;
> 
> If your goal is to catch all instructions, shouldn't you add a counter
> for diagnose functions that don't have a kernel handler as well?

Will add that on top.

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 913c8ac849206..c8b3c1aee7b5c 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -367,6 +367,7 @@ struct kvm_vcpu_stat {
        u64 diagnose_258;
        u64 diagnose_308;
        u64 diagnose_500;
+       u64 diagnose_other;
 };
 
 #define PGM_OPERATION                  0x01
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index 89aa114a2cbad..45634b3d2e0ae 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -257,6 +257,7 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
        case 0x500:
                return __diag_virtio_hypercall(vcpu);
        default:
+               vcpu->stat.diagnose_other++;
                return -EOPNOTSUPP;
        }
 }
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 35e18d84e6828..648c6943cdfed 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -138,6 +138,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
        { "instruction_diag_258", VCPU_STAT(diagnose_258) },
        { "instruction_diag_308", VCPU_STAT(diagnose_308) },
        { "instruction_diag_500", VCPU_STAT(diagnose_500) },
+       { "instruction_diag_other", VCPU_STAT(diagnose_other) },
        { NULL }
 };
 



> 
> I've never used the counters much, but that change looks fine in
> general.

Whenever I have a performance issue, this is the first thing that I look at. :-)




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux