xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH v4 3/3] x86/time: avoid reading the platform timer in rendezvous functions
Date: Tue, 20 Apr 2021 18:12:23 +0200	[thread overview]
Message-ID: <YH79ZxNRhW24jmUS@Air-de-Roger> (raw)
In-Reply-To: <bdf9640d-3c70-461f-6680-9ce883c19719@suse.com>

On Thu, Apr 01, 2021 at 11:55:10AM +0200, Jan Beulich wrote:
> Reading the platform timer isn't cheap, so we'd better avoid it when the
> resulting value is of no interest to anyone.
> The consumer of master_stime, obtained by
> time_calibration_{std,tsc}_rendezvous() and propagated through
> this_cpu(cpu_calibration), is local_time_calibration(). With
> CONSTANT_TSC the latter function uses an early exit path, which doesn't
> explicitly use the field. While this_cpu(cpu_calibration) (including the
> master_stime field) gets propagated to this_cpu(cpu_time).stamp on that
> path, both structures' fields get consumed only by the !CONSTANT_TSC
> logic of the function.
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v4: New.
> ---
> I realize there's some risk associated with potential new uses of the
> field down the road. What would people think about compiling time.c a
> 2nd time into a dummy object file, with a conditional enabled to force
> assuming CONSTANT_TSC, and with that conditional used to suppress
> presence of the field as well as all audited used of it (i.e. in
> particular that large part of local_time_calibration())? Unexpected new
> users of the field would then cause build time errors.

Wouldn't that add quite a lot of churn to the file itself in the form
of pre-processor conditionals?

Could we instead set master_stime to an invalid value that would make
the consumers explode somehow?

I know there might be new consumers, but those should be able to
figure whether the value is sane by looking at the existing ones.

Also, since this is only done on the BSP on the last iteration I
wonder if it really makes such a difference performance-wise to
warrant all this trouble.

Thanks, Roger.

  reply	other threads:[~2021-04-20 16:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01  9:53 [PATCH v4 0/3] x86/time: calibration rendezvous adjustments Jan Beulich
2021-04-01  9:54 ` [PATCH v4 1/3] x86/time: latch to-be-written TSC value early in rendezvous loop Jan Beulich
2021-04-20 15:44   ` Roger Pau Monné
2021-04-01  9:54 ` [PATCH v4 2/3] x86/time: yield to hyperthreads after updating TSC during rendezvous Jan Beulich
2021-04-20 15:59   ` Roger Pau Monné
2021-04-21  9:57     ` Jan Beulich
2021-04-01  9:55 ` [PATCH v4 3/3] x86/time: avoid reading the platform timer in rendezvous functions Jan Beulich
2021-04-20 16:12   ` Roger Pau Monné [this message]
2021-04-21 10:06     ` Jan Beulich
2021-04-29  9:32       ` Jan Beulich
2021-04-29 12:48       ` Roger Pau Monné
2021-04-29 12:53   ` Roger Pau Monné
2021-04-29 13:51     ` Jan Beulich
2021-04-15  9:54 ` Ping: [PATCH v4 0/3] x86/time: calibration rendezvous adjustments Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YH79ZxNRhW24jmUS@Air-de-Roger \
    --to=roger.pau@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).