On Wed, 17 Mar 2021 16:39:04 +0100
Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> On 3/17/21 4:24 PM, 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.
> >
> > With this patch applied:
> >
> > 384 0.518382
> >
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
> > ---
> > target/ppc/kvm.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 298c1f882c67..9ad3dae29132 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -1819,7 +1819,13 @@ uint32_t kvmppc_get_tbfreq(void)
> > {
> > char line[512];
> > char *ns;
> > - uint32_t retval = NANOSECONDS_PER_SECOND;
> > + static uint32_t retval = -1;
>
> Please document why in the code ...
>
Yeah, I've been lazy :)
> > +
> > + if (retval != -1) {
> > + return retval;
> > + }
> > +
> > + retval = NANOSECONDS_PER_SECOND;
> >
> > if (read_cpuinfo("timebase", line, sizeof(line))) {
> > return retval;
> > @@ -1832,7 +1838,8 @@ uint32_t kvmppc_get_tbfreq(void)
> >
> > ns++;
> >
> > - return atoi(ns);
> > + retval = atoi(ns);
> > + return retval;
> > }
>
> ... or alternatively use self-documented code:
>
> -- >8 --
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 298c1f882c6..2b2fe5d8148 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1815,7 +1815,7 @@ 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;
> @@ -1835,6 +1835,17 @@ uint32_t kvmppc_get_tbfreq(void)
> return atoi(ns);
> }
>
> +uint32_t kvmppc_get_tbfreq(void)
> +{
> + static uint32_t cached_tbfreq;
> +
> + if (!cached_tbfreq) {
> + cached_tbfreq = kvmppc_get_tbfreq_procfs();
> + }
> +
> + return cached_tbfreq;
> +}
> +
This is much nicer indeed. I'll do just that in v2.
Thanks ! :)
> bool kvmppc_get_host_serial(char **value)
> {
> return g_file_get_contents("/proc/device-tree/system-id", value, NULL,
> ---
