Re: [Qemu-devel] [PATCH] tcg/monitor: remove "info profile"

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> "info profile" is not great in several ways:
>
> 1) half of it only works for TCG, but doesn't say this anywhere.
>
> 2) the division by get_ticks_per_sec() doesn't work since the unit of
> measurement is clock cycles rather than nanoseconds.  (Broken since 2006
> by Fabrice).
>
> 3) you really can get the same information from "top" now that we have
> VCPU threads.
>
> 4) It declares non-existing extern variables qemu_time_start and
> tlb_flush_time, the latter of which has never existed _at all_ in the
> code base.
>
> Let's remove and leave the remaining TCG dump/profiling functionality that
> is keyed off --enable-profiler.  This "fixes" the conflict between the
> qemu_time() function and the qemu_time variable used by "info profile".

I'd appreciate a brief hint at what the "remaining TCG dump/profiling
functionality" is here.

>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  cpus.c               |  9 ---------
>  include/qemu/timer.h |  4 ----
>  monitor.c            | 28 ----------------------------
>  vl.c                 |  9 ---------
>  4 files changed, 50 deletions(-)
[...]
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index eba8b21..cc74a30 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -1001,10 +1001,6 @@ static inline int64_t profile_getclock(void)
   #ifdef CONFIG_PROFILER
   static inline int64_t profile_getclock(void)
>  {
>      return cpu_get_real_ticks();
>  }
> -
> -extern int64_t qemu_time, qemu_time_start;
> -extern int64_t tlb_flush_time;
> -extern int64_t dev_time;
>  #endif
>  
>  #endif

I can't help to wonder what wrapping profile_getclock() around
cpu_get_real_ticks() buys us.

[...]

With or without the suggested commit message improvement:

Reviewed-by: Markus Armbruster <address@hidden>