Re: [PATCH v1 2/3] KVM: s390: add stat counter for shadow gmap events

[Janosch Frank <frankja@linux.ibm.com>]

On 5/9/23 17:14, Claudio Imbrenda wrote:
> On Tue, 09 May 2023 16:54:21 +0200
> Nico Boehr <nrb@linux.ibm.com> wrote:
> 
>> Quoting Janosch Frank (2023-05-09 13:59:46)
>> [...]
>>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>>>> index 3c3fe45085ec..7f70e3bbb44c 100644
>>>> --- a/arch/s390/include/asm/kvm_host.h
>>>> +++ b/arch/s390/include/asm/kvm_host.h
>>>> @@ -777,6 +777,11 @@ struct kvm_vm_stat {
>>>>        u64 inject_service_signal;
>>>>        u64 inject_virtio;
>>>>        u64 aen_forward;
>>>> +     u64 gmap_shadow_acquire;
>>>> +     u64 gmap_shadow_r2;
>>>> +     u64 gmap_shadow_r3;
>>>> +     u64 gmap_shadow_segment;
>>>> +     u64 gmap_shadow_page;
>>>
>>> This needs to be gmap_shadow_pgt and then we need a separate shadow page
>>> counter that's beeing incremented in kvm_s390_shadow_fault().
>>>
>>>
>>> I'm wondering if we should name them after the entries to reduce
>>> confusion especially when we get huge pages in the future.
>>>
>>> gmap_shadow_acquire
>>> gmap_shadow_r1_te (ptr to r2 table)
>>> gmap_shadow_r2_te (ptr to r3 table)
>>> gmap_shadow_r3_te (ptr to segment table)
>>> gmap_shadow_sg_te (ptr to page table)
>>> gmap_shadow_pg_te (single page table entry)
> 
> but then why not calling them gmap_shadow_{pte,pmd,pud,p4d,pgd} ?
> 

Because I'll need to look up the order of the names after the pmd :)
The gmap mostly works with s390 names.

I'm not totally opposed to that but I also don't see a clear benefit.