mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dan Magenheimer <dan.magenheimer@oracle.com>
To: Jan Beulich <JBeulich@suse.com>, David Vrabel <david.vrabel@citrix.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	xen-devel <xen-devel@lists.xen.org>,
	Konrad Wilk <konrad.wilk@oracle.com>,
	linux-kernel@vger.kernel.org, "Tim (Xen.org)" <tim@xen.org>,
	Sheng Yang <sheng@yasker.org>
Subject: RE: [Xen-devel] [PATCH] xen: always set the sched clock as unstable
Date: Mon, 16 Apr 2012 10:22:08 -0700 (PDT)	[thread overview]
Message-ID: <84023b26-a682-44b5-990e-1635da6949ff@default> (raw)
In-Reply-To: <4F8C619C020000780007E34E@nat28.tlf.novell.com>

> From: Jan Beulich [mailto:JBeulich@suse.com]
> Subject: RE: [Xen-devel] [PATCH] xen: always set the sched clock as unstable
> 
> >>> On 16.04.12 at 18:05, Dan Magenheimer <dan.magenheimer@oracle.com> wrote:
> >>  From: David Vrabel [mailto:david.vrabel@citrix.com]
> >> On 16/04/12 12:32, Jan Beulich wrote:
> >> >>>> On 13.04.12 at 20:20, David Vrabel <david.vrabel@citrix.com> wrote:
> >> >> --- a/arch/x86/xen/time.c
> >> >> +++ b/arch/x86/xen/time.c
> >> >> @@ -473,6 +473,7 @@ static void __init xen_time_init(void)
> >> >>  	do_settimeofday(&tp);
> >> >>
> >> >>  	setup_force_cpu_cap(X86_FEATURE_TSC);
> >> >> +	sched_clock_stable = 0;
> >> >
> >> > This, unfortunately, is not sufficient afaict: If a CPU gets brought up
> >> > post-boot, the variable may need to be cleared again. Instead you
> >> > ought to call mark_tsc_unstable().
> >>
> >> Yeah, mark_tsc_unstable() is the right thing to do.
> >
> > NACK!
> >
> > No, no, no.  The exact opposite is true.  Like VMware, TSC is
> > stable.  The issue is that Linux trusts other clock hardware more
> > completely than TSC so whenever there is a problem with another
> > clocksource, Linux blames TSC and marks TSC unstable.  But TSC
> > on Xen 4.0+ is innocent.  In fact, TSC is a better clocksource
> > choice than clocksource=xen (aka pvclock) because pvclock
> > indirectly depends on TSC.
> >
> > For upstream kernels, the answer is to set clocksource=tsc
> > and tsc=reliable, like VMware enforces. See:
> >
> > https://lists.ubuntu.com/archives/kernel-team/2008-October/004283.html
> >
> > In fact, it might be wise for a Xen-savvy kernel to check to see
> > if it is running on Xen-4.0+ and, if so, force clocksource=tsc
> > and tsc=reliable.
> 
> Are you possibly mixing up PV and HVM cases? sched_clock_stable
> getting set _is_ a problem in PV kernels - we had bug reports long
> ago when this first appeared in arch/x86/kernel/cpu/intel.c. I'm
> suspecting this because there's not supposed to be (and in non-
> pv-ops there is no; in pv-ops I assume it simply has no effect)
> clocksource=tsc in PV kernels.

Hi Jan --

In upstream (and recent pv-ops) kernels, is there any need for there
to be a difference between HVM and PV in the clocksource chosen?  The
pvclock algorithm was necessary for PV when non-TSC hardware clocks
were privileged and the only non-privileged hardware clock (TSC)
was badly broken in hardware and for migration/save/restore.
With TSC now working and stable, and now that we are making changes
in the upstream kernel that work for both PV and HVM, is it
time to drop pvclock (at least as the default for PV)?

Certainly if an old (non-pv-ops) kernel is broken, something like
David's patch might be an acceptable workaround.  I'm just arguing
against perpetuating pvclock-as-the-only-xen-clock upstream.

Does that make sense?
Dan

  reply	other threads:[~2012-04-16 17:22 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-13 18:20 David Vrabel
2012-04-13 18:33 ` Sheng Yang
     [not found] ` <CA+2rt41ctpU-vhR7=u_r45=o8djpq0-5YE5jcj_4FR2bWYu5pQ@mail.gmail.com>
2012-04-13 18:39   ` David Vrabel
2012-04-16 11:32 ` Jan Beulich
2012-04-16 14:59   ` David Vrabel
2012-04-16 15:16     ` Tim Deegan
2012-04-16 15:17     ` Konrad Rzeszutek Wilk
2012-04-16 16:20       ` [Xen-devel] " Dan Magenheimer
2012-04-16 16:05     ` Dan Magenheimer
2012-04-16 16:14       ` Jan Beulich
2012-04-16 17:22         ` Dan Magenheimer [this message]
2012-04-17  7:27           ` Jan Beulich
2012-04-17 15:36             ` Dan Magenheimer
2012-04-16 16:26       ` David Vrabel
2012-04-16 17:30         ` Dan Magenheimer
2012-04-17  7:47           ` Jan Beulich
2012-04-17 15:42             ` Dan Magenheimer
2012-04-16 17:08       ` Tim Deegan
2012-04-16 17:52         ` Dan Magenheimer
2012-04-16 18:17           ` Tim Deegan
2012-04-16 23:01             ` Sheng Yang
2012-04-17  0:29               ` Dan Magenheimer
2012-04-17  8:19               ` Tim Deegan

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=84023b26-a682-44b5-990e-1635da6949ff@default \
    --to=dan.magenheimer@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=david.vrabel@citrix.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sheng@yasker.org \
    --cc=tglx@linutronix.de \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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

all inboxes | Powered by JetHome®