On 10.10.19 12:25, David Hildenbrand wrote:
> On 10.10.19 12:21, Christian Borntraeger wrote:
>> To analyze some performance issues with lock contention and scheduling
>> it is nice to know when diag9c did not result in any action.
>> At the same time avoid calling the scheduler (which can be expensive)
>> if we know that it does not make sense.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> ---
>> arch/s390/include/asm/kvm_host.h | 2 ++
>> arch/s390/kvm/diag.c | 23 +++++++++++++++++++----
>> arch/s390/kvm/kvm-s390.c | 2 ++
>> 3 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index abe60268335d..743cd5a63b37 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -392,6 +392,8 @@ struct kvm_vcpu_stat {
>> u64 diagnose_10;
>> u64 diagnose_44;
>> u64 diagnose_9c;
>> + u64 diagnose_9c_success;
>> + u64 diagnose_9c_ignored;
>
> Can't you derive the one from the other with diagnose_9c? Just sayin,
> one would be sufficient.
You mean diagnose9c = diagnose_9c_success + diagnose_9c_ignored anyway so this is redundant?
Could just do diagnose_9c and diagnose_9c_ignored if you prefer that.
>
>> u64 diagnose_258;
>> u64 diagnose_308;
>> u64 diagnose_500;
>> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
>> index 45634b3d2e0a..2c729f020585 100644
>> --- a/arch/s390/kvm/diag.c
>> +++ b/arch/s390/kvm/diag.c
>> @@ -158,14 +158,29 @@ static int __diag_time_slice_end_directed(struct kvm_vcpu *vcpu)
>>
>> tid = vcpu->run->s.regs.gprs[(vcpu->arch.sie_block->ipa & 0xf0) >> 4];
>> vcpu->stat.diagnose_9c++;
>> - VCPU_EVENT(vcpu, 5, "diag time slice end directed to %d", tid);
>>
>> + /* yield to self */
>> if (tid == vcpu->vcpu_id)
>> - return 0;
>> + goto no_yield;
>>
>> + /* yield to invalid */
>> tcpu = kvm_get_vcpu_by_id(vcpu->kvm, tid);
>> - if (tcpu)
>> - kvm_vcpu_yield_to(tcpu);
>> + if (!tcpu)
>> + goto no_yield;
>> +
>> + /* target already running */
>> + if (tcpu->cpu >= 0)
>> + goto no_yield;
>
> Wonder if it's wort moving this optimization to a separate patch.
Could do if you prefer that.
>
>> +
>> + if (kvm_vcpu_yield_to(tcpu) <= 0)
>> + goto no_yield;
>> +
>> + VCPU_EVENT(vcpu, 5, "diag time slice end directed to %d: done", tid);
>> + vcpu->stat.diagnose_9c_success++;
>> + return 0;
>> +no_yield:
>> + VCPU_EVENT(vcpu, 5, "diag time slice end directed to %d: ignored", tid);
>> + vcpu->stat.diagnose_9c_ignored++;
>> return 0;
>> }
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 4a3bc40ca1a4..b368e835f2f7 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -155,6 +155,8 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>> { "instruction_diag_10", VCPU_STAT(diagnose_10) },
>> { "instruction_diag_44", VCPU_STAT(diagnose_44) },
>> { "instruction_diag_9c", VCPU_STAT(diagnose_9c) },
>> + { "diag_9c_success", VCPU_STAT(diagnose_9c_success) },
>> + { "diag_9c_ignored", VCPU_STAT(diagnose_9c_ignored) },
>> { "instruction_diag_258", VCPU_STAT(diagnose_258) },
>> { "instruction_diag_308", VCPU_STAT(diagnose_308) },
>> { "instruction_diag_500", VCPU_STAT(diagnose_500) },
>>
>
>
