From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Fitzhardinge Subject: Re: [PATCH] x86: unconditionally mark TSC unstable under Xen Date: Wed, 14 Jul 2010 14:36:12 -0700 Message-ID: <4C3E2DCC.1010201@goop.org> References: <51C8308F-F413-47AF-8845-C92BD36CA35C@linode.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51C8308F-F413-47AF-8845-C92BD36CA35C@linode.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jed Smith Cc: xen-devel@lists.xensource.com, Jan Beulich List-Id: xen-devel@lists.xenproject.org On 07/14/2010 12:24 PM, Jed Smith wrote: > Jeremy, Jan - what do you think? Is this a bad move? I feel like there > is a consequence to this that I am unaware of, but it fixes my issue. > Ah, well that's interesting. There's a couple of comments: 1. you can't do this with just a compile-time test, since the same kernel can also boot native 2. nothing in a Xen PV domU environment should be using the tsc directly, so this shouldn't have an effect. If something is using the tsc we should track it down. I wonder, however, if you're getting the same result as Jan's suggestion of making sched_clock unstable by making the tsc unstable. In that case, this patch may help: Subject: [PATCH] xen: disable xen_sched_clock by default xen_sched_clock only counts unstolen time. In principle this should be useful to the Linux scheduler so that it knows how much time a process actually consumed. But in practice this doesn't work very well as the scheduler expects the sched_clock time to be synchronized between cpus. It also uses sched_clock to measure the time a task spends sleeping, in which case "unstolen time" isn't meaningful. So just use plain xen_clocksource_read to return wallclock nanoseconds for sched_clock. Signed-off-by: Jeremy Fitzhardinge diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig index b83e119..6a09f01 100644 --- a/arch/x86/xen/Kconfig +++ b/arch/x86/xen/Kconfig @@ -29,6 +29,10 @@ config XEN_SAVE_RESTORE depends on XEN && PM default y +config XEN_SCHED_CLOCK + bool + default n + config XEN_DEBUG_FS bool "Enable Xen debug and tuning parameters in debugfs" depends on XEN && DEBUG_FS diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index 4b57c0b..242a230 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -939,7 +939,11 @@ static const struct pv_time_ops xen_time_ops __initdata = { .set_wallclock = xen_set_wallclock, .get_wallclock = xen_get_wallclock, .get_tsc_khz = xen_tsc_khz, +#ifdef CONFIG_XEN_SCHED_CLOCK .sched_clock = xen_sched_clock, +#else + .sched_clock = xen_clocksource_read, +#endif }; static const struct pv_cpu_ops xen_cpu_ops __initdata = { diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c index 9d1f853..32dc3dc 100644 --- a/arch/x86/xen/time.c +++ b/arch/x86/xen/time.c @@ -154,6 +154,7 @@ static void do_stolen_accounting(void) account_idle_ticks(ticks); } +#ifdef CONFIG_XEN_SCHED_CLOCK /* * Xen sched_clock implementation. Returns the number of unstolen * nanoseconds, which is nanoseconds the VCPU spent in RUNNING+BLOCKED @@ -191,7 +192,7 @@ unsigned long long xen_sched_clock(void) return ret; } - +#endif /* Get the TSC speed from Xen */ unsigned long xen_tsc_khz(void) Thanks, J