Re: [PATCH v2] target/ppc/kvm: Cache timebase frequency

[David Gibson]

On Wed, Mar 17, 2021 at 06:57:07PM +0100, Greg Kurz wrote:
> Each vCPU core exposes its timebase frequency in the DT. When running
> under KVM, this means parsing /proc/cpuinfo in order to get the timebase
> frequency of the host CPU.
> 
> The parsing appears to slow down the boot quite a bit with higher number
> of cores:
> 
> # of cores     seconds spent in spapr_dt_cpus()
>       8                  0.550122
>      16                  1.342375
>      32                  2.850316
>      64                  5.922505
>      96                  9.109224
>     128                 12.245504
>     256                 24.957236
>     384                 37.389113
> 
> The timebase frequency of the host CPU is identical for all
> cores and it is an invariant for the VM lifetime. Cache it
> instead of doing the same expensive parsing again and again.
> 
> Rename kvmppc_get_tbfreq() to kvmppc_get_tbfreq_procfs() and
> rename the 'retval' variable to make it clear it is used as
> fallback only. Come up with a new version of kvmppc_get_tbfreq()
> that calls kvmppc_get_tbfreq_procfs() only once and keep the
> value in a static.
> 
> Zero is certainly not a valid value for the timebase frequency.
> Treat atoi() returning zero as another parsing error and return
> the fallback value instead. This allows kvmppc_get_tbfreq() to
> use zero as an indicator that kvmppc_get_tbfreq_procfs() hasn't
> been called yet.
> 
> With this patch applied:
> 
>     384                 0.518382
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-6.0, thanks.

> ---
> v2: - do the caching in a distinct function for clarity (Philippe)
>     - rename 'retval' to 'tbfreq_fallback'
>     - expand the changelog a bit
> ---
>  target/ppc/kvm.c |   25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 298c1f882c67..104a308abb57 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1815,24 +1815,37 @@ static int read_cpuinfo(const char *field, char 
> *value, int len)
>      return ret;
>  }
>  
> -uint32_t kvmppc_get_tbfreq(void)
> +static uint32_t kvmppc_get_tbfreq_procfs(void)
>  {
>      char line[512];
>      char *ns;
> -    uint32_t retval = NANOSECONDS_PER_SECOND;
> +    uint32_t tbfreq_fallback = NANOSECONDS_PER_SECOND;
> +    uint32_t tbfreq_procfs;
>  
>      if (read_cpuinfo("timebase", line, sizeof(line))) {
> -        return retval;
> +        return tbfreq_fallback;
>      }
>  
>      ns = strchr(line, ':');
>      if (!ns) {
> -        return retval;
> +        return tbfreq_fallback;
>      }
>  
> -    ns++;
> +    tbfreq_procfs = atoi(++ns);
> +
> +    /* 0 is certainly not acceptable by the guest, return fallback value */
> +    return tbfreq_procfs ? tbfreq_procfs : tbfreq_fallback;
> +}
> +
> +uint32_t kvmppc_get_tbfreq(void)
> +{
> +    static uint32_t cached_tbfreq;
> +
> +    if (!cached_tbfreq) {
> +        cached_tbfreq = kvmppc_get_tbfreq_procfs();
> +    }
>  
> -    return atoi(ns);
> +    return cached_tbfreq;
>  }
>  
>  bool kvmppc_get_host_serial(char **value)
> 
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature