Re: [PATCH] accel/kvm: Turn DPRINTF macro use into tracepoints

[Peter Maydell]

On Mon, 27 Nov 2023 at 19:44, Jai Arora <arorajai2798@gmail.com> wrote:
>
> To remove DPRINTF macros and use tracepoints
> for logging.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1827
>
> Signed-off-by: Jai Arora <arorajai2798@gmail.com>
> ---
>  accel/kvm/kvm-all.c    | 32 ++++++++++----------------------
>  accel/kvm/trace-events |  2 +-
>  2 files changed, 11 insertions(+), 23 deletions(-)

Hi; thanks for sending in this patch.

(I've CC'd the KVM maintainer.)

> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index e39a810a4e..d0dd7e54c3 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -69,16 +69,6 @@
>  #define KVM_GUESTDBG_BLOCKIRQ 0
>  #endif
>
> -//#define DEBUG_KVM
> -
> -#ifdef DEBUG_KVM
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> -#endif
> -
>  struct KVMParkedVcpu {
>      unsigned long vcpu_id;
>      int kvm_fd;
> @@ -331,7 +321,7 @@ static int do_kvm_destroy_vcpu(CPUState *cpu)
>      struct KVMParkedVcpu *vcpu = NULL;
>      int ret = 0;
>
> -    DPRINTF("kvm_destroy_vcpu\n");
> +    trace_kvm_dprintf("kvm_destroy_vcpu\n");

Rather than using a single tracepoint for every line
that was previously a DPRINTF, it's preferable to
use tracepoints whose name indicates what they're doing.
Users can turn tracepoints on and off individually, and they
might, for instance, only want to trace when the vcpu is
destroyed and not also the very large number of events
that will happen for kvm_cpu_exec entry/exits.

If you look at the current set of events in accel/kvm/trace-events
you can see the general style: the tracepoint name itself
tells you the "what has happened", the arguments provide
more info (eg which CPU did we just destroy), and it's
rarely necessary to pass a string into the tracepoint function.

thanks
-- PMM