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

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

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} ?

> 
> Yep, right, this was highly confusing to the point where I was also
> confused by it. Will change that, thanks.