Re: [PATCH] Hyper-V: fix for unwanted manipulation of sched_clock when TSC marked unstable
From: Wei Liu
Date: Tue Jul 13 2021 - 11:47:28 EST
On Tue, Jul 13, 2021 at 09:01:06PM +0530, Ani Sinha wrote:
>
>
> On Tue, 13 Jul 2021, Peter Zijlstra wrote:
>
> > On Tue, Jul 13, 2021 at 01:04:46PM +0000, Wei Liu wrote:
> > > On Tue, Jul 13, 2021 at 08:35:21AM +0530, Ani Sinha wrote:
> > > > Marking TSC as unstable has a side effect of marking sched_clock as
> > > > unstable when TSC is still being used as the sched_clock. This is not
> > > > desirable. Hyper-V ultimately uses a paravirtualized clock source that
> > > > provides a stable scheduler clock even on systems without TscInvariant
> > > > CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> > > > scheduler clock has been changed to the paravirtualized clocksource. This
> > > > will prevent any unwanted manipulation of the sched_clock. Only TSC will
> > > > be correctly marked as unstable.
> > >
> > > Correct me if I'm wrong, what you're trying to address is that
> > > sched_clock remains marked as unstable even after Linux has switched to
> > > a stable clock source.
> > >
> > > I think a better approach will be to mark the sched_clock as stable when
> > > we switch to the paravirtualized clock source.
> >
> > No.. unstable->stable transitions are unsound. You get to switch to your
> > paravirt clock earlier.
> >
>
> I believe manipulating sched_clock was never the intention of the original
> author who added the code to mark tsc as unstable on hyper-V:
>
> commit 88c9281a9fba67636ab26c1fd6afbc78a632374f
> Author: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> Date: Wed Aug 19 09:54:24 2015 -0700
>
> x86/hyperv: Mark the Hyper-V TSC as unstable
>
>
> The original author simply wanted to mark TSC as unstable on hyper-V
> systems because of reasons the above commit log will describe. Sched clock
> manipulation happened accidentally because from where the
> mark_tsc_unstable() was being called. This patch simply fixes this.
>
> Michael Kelly from Microsoft has tested this patch already.
OK. In light of Peter's comment and this one, I'm fine with this patch.
Michael, can you give an ack or review here?
Thanks,
Wei.
>
> --Ani
>