* [PATCH] target/ppc/kvm: Cache timebase frequency
@ 2021-03-17 15:24 Greg Kurz
2021-03-17 15:39 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kurz @ 2021-03-17 15:24 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, David Gibson
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 <groug@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;
+
+ 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;
}
bool kvmppc_get_host_serial(char **value)
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] target/ppc/kvm: Cache timebase frequency
2021-03-17 15:24 [PATCH] target/ppc/kvm: Cache timebase frequency Greg Kurz
@ 2021-03-17 15:39 ` Philippe Mathieu-Daudé
2021-03-17 16:32 ` Greg Kurz
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-17 15:39 UTC (permalink / raw)
To: Greg Kurz, qemu-devel; +Cc: qemu-ppc, David Gibson
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 <groug@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 ...
> +
> + 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;
+}
+
bool kvmppc_get_host_serial(char **value)
{
return g_file_get_contents("/proc/device-tree/system-id", value, NULL,
---
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] target/ppc/kvm: Cache timebase frequency
2021-03-17 15:39 ` Philippe Mathieu-Daudé
@ 2021-03-17 16:32 ` Greg Kurz
0 siblings, 0 replies; 3+ messages in thread
From: Greg Kurz @ 2021-03-17 16:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-ppc, qemu-devel, David Gibson
On Wed, 17 Mar 2021 16:39:04 +0100
Philippe Mathieu-Daudé <f4bug@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 <groug@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,
> ---
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-17 16:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-17 15:24 [PATCH] target/ppc/kvm: Cache timebase frequency Greg Kurz
2021-03-17 15:39 ` Philippe Mathieu-Daudé
2021-03-17 16:32 ` Greg Kurz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).