On Mon, 3 Feb 2020 08:19:54 -0500
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
> From: Janosch Frank <frankja@xxxxxxxxxxxxx>
>
> Let's have some debug traces which stay around for longer than the
> guest.
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> ---
> arch/s390/kvm/kvm-s390.c | 10 +++++++++-
> arch/s390/kvm/kvm-s390.h | 9 +++++++++
> arch/s390/kvm/pv.c | 21 +++++++++++++++++++--
> 3 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 2beb93f0572f..d4dc156e2c3e 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -221,6 +221,7 @@ static struct kvm_s390_vm_cpu_subfunc kvm_s390_available_subfunc;
> static struct gmap_notifier gmap_notifier;
> static struct gmap_notifier vsie_gmap_notifier;
> debug_info_t *kvm_s390_dbf;
> +debug_info_t *kvm_s390_dbf_uv;
>
> /* Section: not file related */
> int kvm_arch_hardware_enable(void)
> @@ -462,7 +463,13 @@ int kvm_arch_init(void *opaque)
> if (!kvm_s390_dbf)
> return -ENOMEM;
>
> - if (debug_register_view(kvm_s390_dbf, &debug_sprintf_view))
> + kvm_s390_dbf_uv = debug_register("kvm-uv", 32, 1, 7 * sizeof(long));
> + if (!kvm_s390_dbf_uv)
> + return -ENOMEM;
Doesn't that leak kvm_s390_dbf?
> +
> +
One blank line should be enough.
> + if (debug_register_view(kvm_s390_dbf, &debug_sprintf_view) ||
> + debug_register_view(kvm_s390_dbf_uv, &debug_sprintf_view))
> goto out;
>
> kvm_s390_cpu_feat_init();
> @@ -489,6 +496,7 @@ void kvm_arch_exit(void)
> {
> kvm_s390_gib_destroy();
> debug_unregister(kvm_s390_dbf);
> + debug_unregister(kvm_s390_dbf_uv);
> }
>
> /* Section: device related */
(...)
> @@ -252,7 +269,7 @@ int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
> addr += PAGE_SIZE;
> tw[1] += PAGE_SIZE;
> }
> - VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished rc %x", rc);
> + VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished with rc %x", rc);
Can you merge this into the patch that introduces this log entry?
Also, do you want to add logging into the new dbf here as well?
> return rc;
> }
>
You often seem to log in pairs (into the per-vm dbf and into the new uv
dbf). Would it make sense to introduce a new helper for that, or is
that overkill?
