[v2,0/2] KVM: s390: add counters for vsie performance

Message

Nico Boehr May 10, 2023, 12:18 p.m. UTC

v2:
---
* also count shadowing of pages (Janosch)
* fix naming of counters (Janosch)
* mention shadowing of multiple levels is counted in each level (Claudio)
* fix inaccuate commit description regarding gmap notifier (Claudio)

When running a guest-3 via VSIE, guest-1 needs to shadow the page table
structures of guest-2.

To reflect changes of the guest-2 in the _shadowed_ page table structures,
the _shadowing_ sturctures sometimes need to be rebuilt. Since this is a
costly operation, it should be avoided whenever possible.

This series adds kvm stat counters to count the number of shadow gmaps
created and a tracepoint whenever something is unshadowed. This is a first
step to try and improve VSIE performance.

Please note that "KVM: s390: add tracepoint in gmap notifier" has some
checkpatch --strict findings. I did not fix these since the tracepoint
definition would then look completely different from all the other
tracepoints in arch/s390/kvm/trace-s390.h. If you want me to fix that,
please let me know.

While developing this, a question regarding the stat counters came up:
there's usually no locking involved when the stat counters are incremented.
On s390, GCC accidentally seems to do the right thing(TM) most of the time
by generating a agsi instruction (which should be atomic given proper
alignment). However, it's not guaranteed, so would we rather want to go
with an atomic for the stat counters to avoid losing events? Or do we just
accept the fact that we might loose events sometimes? Is there anything
that speaks against having an atomic in kvm_stat?

Nico Boehr (2):
  KVM: s390: add stat counter for shadow gmap events
  KVM: s390: add tracepoint in gmap notifier

 arch/s390/include/asm/kvm_host.h |  6 ++++++
 arch/s390/kvm/gaccess.c          |  7 +++++++
 arch/s390/kvm/kvm-s390.c         | 10 +++++++++-
 arch/s390/kvm/trace-s390.h       | 23 +++++++++++++++++++++++
 arch/s390/kvm/vsie.c             |  1 +
 5 files changed, 46 insertions(+), 1 deletion(-)

Comments

Nico Boehr July 27, 2023, 7:24 a.m. UTC | #1

Polite ping.

Quoting Nico Boehr (2023-05-10 14:18:20)
> v2:
> ---
> * also count shadowing of pages (Janosch)
> * fix naming of counters (Janosch)
> * mention shadowing of multiple levels is counted in each level (Claudio)
> * fix inaccuate commit description regarding gmap notifier (Claudio)
> 
> When running a guest-3 via VSIE, guest-1 needs to shadow the page table
> structures of guest-2.
> 
> To reflect changes of the guest-2 in the _shadowed_ page table structures,
> the _shadowing_ sturctures sometimes need to be rebuilt. Since this is a
> costly operation, it should be avoided whenever possible.
> 
> This series adds kvm stat counters to count the number of shadow gmaps
> created and a tracepoint whenever something is unshadowed. This is a first
> step to try and improve VSIE performance.
> 
> Please note that "KVM: s390: add tracepoint in gmap notifier" has some
> checkpatch --strict findings. I did not fix these since the tracepoint
> definition would then look completely different from all the other
> tracepoints in arch/s390/kvm/trace-s390.h. If you want me to fix that,
> please let me know.
> 
> While developing this, a question regarding the stat counters came up:
> there's usually no locking involved when the stat counters are incremented.
> On s390, GCC accidentally seems to do the right thing(TM) most of the time
> by generating a agsi instruction (which should be atomic given proper
> alignment). However, it's not guaranteed, so would we rather want to go
> with an atomic for the stat counters to avoid losing events? Or do we just
> accept the fact that we might loose events sometimes? Is there anything
> that speaks against having an atomic in kvm_stat?
> 
> Nico Boehr (2):
>   KVM: s390: add stat counter for shadow gmap events
>   KVM: s390: add tracepoint in gmap notifier
> 
>  arch/s390/include/asm/kvm_host.h |  6 ++++++
>  arch/s390/kvm/gaccess.c          |  7 +++++++
>  arch/s390/kvm/kvm-s390.c         | 10 +++++++++-
>  arch/s390/kvm/trace-s390.h       | 23 +++++++++++++++++++++++
>  arch/s390/kvm/vsie.c             |  1 +
>  5 files changed, 46 insertions(+), 1 deletion(-)
> 
> -- 
> 2.39.1
>