Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 28 Nov 2022 10:15:43 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     "Paul E. McKenney" <paulmck@kernel.org>
CC:     Thomas Gleixner <tglx@linutronix.de>,
        <linux-kernel@vger.kernel.org>, <john.stultz@linaro.org>,
        <sboyd@kernel.org>, <corbet@lwn.net>, <Mark.Rutland@arm.com>,
        <maz@kernel.org>, <kernel-team@meta.com>, <neeraju@codeaurora.org>,
        <ak@linux.intel.com>, <zhengjun.xing@intel.com>,
        Chris Mason <clm@meta.com>, John Stultz <jstultz@google.com>,
        Waiman Long <longman@redhat.com>
Subject: Re: [PATCH clocksource 1/3] clocksource: Reject bogus watchdog
 clocksource measurements
Message-ID: <Y4QZzzk+FdGj4AXm@feng-clx>
References: <20221114232807.GA834337@paulmck-ThinkPad-P17-Gen-1>
 <20221114232827.835599-1-paulmck@kernel.org>
 <87mt8pkzw1.ffs@tglx>
 <20221117230910.GI4001@paulmck-ThinkPad-P17-Gen-1>
 <Y3rMc2VbgVLHN9db@feng-clx>
 <20221121181449.GA3774542@paulmck-ThinkPad-P17-Gen-1>
 <Y3zxB6r1kin8pSH1@feng-clx>
 <20221122220712.GP4001@paulmck-ThinkPad-P17-Gen-1>
 <Y32HFE+BaATh9+l/@feng-clx>
 <20221123212348.GI4001@paulmck-ThinkPad-P17-Gen-1>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221123212348.GI4001@paulmck-ThinkPad-P17-Gen-1>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MNNjnhyvb5QxAiODsjKYXnwQsnyiQkO0AjHsgwzhN7XucWUWf1YcEOhmipEw?=
 =?us-ascii?Q?A2Gi9Yx6KWbcwDMgUEgKTGGZREmb5xjinIdWkfrGl6BEEqBiAISUeURY6jLV?=
 =?us-ascii?Q?w6wsJCtrxDstFgO1i+3wuOuMhVsfbSs1nFt/h2RydDasuQzbCUArqHeMohIa?=
 =?us-ascii?Q?Vpl/2hzmfh/JhD2DMhovBmFhXjvptkR7Lq85Eqt8KKJBWT8xEPz75HDtN8BK?=
 =?us-ascii?Q?0IV4HGFy9GyWmE0fjWsARZh35Vehs/GoKfH71CWwoq52B/iKbMVR3m7DQovz?=
 =?us-ascii?Q?FKHJGDBTpaEAgQpiiqe+5aIQHsLUUNDk0bm3D2acWC21R9MSv1b8QnCaGL/6?=
 =?us-ascii?Q?v+QRdnJBddcn9ibpqfAF/Ho4Zek/1aujWcAtgiRTVqkAgQ8feUnZtkWrgK8H?=
 =?us-ascii?Q?i6vJp3nlW3Tz1n4/bjA3hUncIMyopGIXoIipeepapHPMer8wpiniN8mjN2RQ?=
 =?us-ascii?Q?p9YONNDhRi3OJas29p/B0x2ILtRNdd76TeUN2HV9aSqcV88G7tXrUoJTQilu?=
 =?us-ascii?Q?9V3U/GdKPMGprgY9V/d6yjoRkCXBYEUAOJwyqXrMFH7UAw5Qc3jv7Cg4sN4f?=
 =?us-ascii?Q?89v3EKzk6pKNNBmDtNn2vI5pwk/P4bKJ6jW+6OI0MZR5lD/QTUtZ7HXifrHa?=
 =?us-ascii?Q?bVvpiUSgkghJIlt1pdgiMBHZTbvPljCtwaIjjTzf75pEFle1fWRnABp/SB5/?=
 =?us-ascii?Q?DkKVDnsEARC86H6myLhLDcZLmy1Ot0FDoQRWwnk62S1HLKkcnVc32ebdV+9Z?=
 =?us-ascii?Q?dvkZT9IYY8yqjkuLcNERZs2yczi3nJFihQQUK9FDBbpoF+2P8/9BqiJ6EJcx?=
 =?us-ascii?Q?G2KmINZ/odPg2tLCPulEPnYzrhRngxuS4c0RF1Bizl/SgPdkdk0zBZkHKnu0?=
 =?us-ascii?Q?KY4qczYYYGu51bZ2HAM9Ayi43HJKA/iBgyLzGNGDcDYK3qTWjUHkeFQoqKj2?=
 =?us-ascii?Q?QofOPf5VESnpxpl0IomJbziPkrE0Ezo9xwcbbIuUW+ehvEvasjfx/T2qGUVA?=
 =?us-ascii?Q?W3mnlzeR2qdfC2XgqKaXhbVCCDd3GsAol1J30i5b5zXQccJzkKiRCwxoNZVR?=
 =?us-ascii?Q?KODM7NqKmyFRG4g9PLLlMWg/RbNIHxfTaW8IPh9t2HqWqqA9ulzTXsPIgzWH?=
 =?us-ascii?Q?sNacy6TC1yXqWQ97pG4RrW2HQsnuRhPyQVDE3sKz65bZjrDFkKZHD/58NRC6?=
 =?us-ascii?Q?C1ch7r5ZFn2be0O9ju+BeSgUYc61ywtGOeYSU5oTY1G1bB/zTt+WmbM/Mmd1?=
 =?us-ascii?Q?WZKpdc9CLxd2mhDCXDeWZ99D+Dn3TyoiW2K4oPrejhQLF+nJ0a2GYvvMZGam?=
 =?us-ascii?Q?z/JJJ7ueF4Bc9NCfORkcE4+Za3L5gNoDdhGy3krAOXBN/WIzlVW27x1JURqg?=
 =?us-ascii?Q?pl5NlvhVVY3WF/4AfFAAjLCOk/lPrti8MCVG5fmWo7jatKQgdGUMaL5W4ZyG?=
 =?us-ascii?Q?VaSkiabZtrXXNB72dYBDWNRdyOT0mVLmvnGZUuHU8+9lHob6l1x25WeE55Uw?=
 =?us-ascii?Q?LORHZPW2dZmNkQdVwTbWgZuk8/ewcBQjQJBE5oltiOH/kreihMkzU9wKkzFs?=
 =?us-ascii?Q?aMC8zu8QF1lOoE3ChD58SbAzdCCDqViTwgsnTU7T?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6a117bbf-aa67-4055-85ff-08dad0e6e320
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Nov 2022 02:18:49.9824
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: axNNfUS+WjIO6+9d+H9N38js4CJOCCtdp9gG+fvndLlp4a+gODYarhbUKB7myJjCTh1iOuMmQR7Akkqp0sscnQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5868
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Nov 23, 2022 at 01:23:48PM -0800, Paul E. McKenney wrote:
> On Wed, Nov 23, 2022 at 10:36:04AM +0800, Feng Tang wrote:
> > On Tue, Nov 22, 2022 at 02:07:12PM -0800, Paul E. McKenney wrote:
> > [...] 
> > > > > If PM_TIMER was involved, I would expect 'acpi_pm' instead of
> > > > > refined-jiffies.  Or am I misinterpreting the output and/or code?
> > > > 
> > > > It's about timing. On a typical server platform, the clocksources
> > > > init order could be:
> > > >   refined-jiffies --> hpet --> tsc-early --> acpi_pm --> tsc 
> > > > 
> > > > >From your log, TSC('tsc-early') is disabled before 'acpi_pm' get
> > > > initialized, so 'acpi_pm' timer (if exist) had no chance to watchdog
> > > > the tsc.
> > > > 
> > > > > Either way, would it make sense to add CLOCK_SOURCE_MUST_VERIFY to
> > > > > clocksource_hpet.flags?
> > > > 
> > > > Maybe try below patch, which will skip watchdog for 'tsc-early',
> > > > while giving 'acpi_pm' timer a chance to watchdog 'tsc'.
> > > > 
> > > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > > > index cafacb2e58cc..9840f0131764 100644
> > > > --- a/arch/x86/kernel/tsc.c
> > > > +++ b/arch/x86/kernel/tsc.c
> > > > @@ -1131,8 +1131,7 @@ static struct clocksource clocksource_tsc_early = {
> > > >  	.uncertainty_margin	= 32 * NSEC_PER_MSEC,
> > > >  	.read			= read_tsc,
> > > >  	.mask			= CLOCKSOURCE_MASK(64),
> > > > -	.flags			= CLOCK_SOURCE_IS_CONTINUOUS |
> > > > -				  CLOCK_SOURCE_MUST_VERIFY,
> > > > +	.flags			= CLOCK_SOURCE_IS_CONTINUOUS,
> > > >  	.vdso_clock_mode	= VDSO_CLOCKMODE_TSC,
> > > >  	.enable			= tsc_cs_enable,
> > > >  	.resume			= tsc_resume,
> > > 
> > > Your mainline patch b50db7095fe0 ("x86/tsc: Disable clocksource watchdog
> > > for TSC on qualified platorms") mitigates the issue so we are good for
> > > the immediate future, at least assuming reliable TSC.
> > > 
> > > But it also disables checking against HPET, hence my question about
> > > marking clocksource_hpet.flags with CLOCK_SOURCE_MUST_VERIFY at boot time
> > > on systems whose CPUs have constant_tsc, nonstop_tsc, and tsc_adjust.
> > 
> > IIUC, this will make TSC to watchdog HPET every 500 ms. We have got
> > report that the 500ms watchdog timer had big impact on some parallel
> > workload on big servers, that was another factor for us to seek
> > stopping the timer.
> 
> Another approach would be to slow it down.  Given the tighter bounds
> on skew, it could be done every (say) 10 seconds while allowing
> 2 milliseconds skew instead of the current 100 microseconds.

Yes, this can reduce the OS noise much. One problem is if we make it
a general interface, there is some clocksource whose warp time is
less than 10 seconds, like ACPI PM_TIMER (3-4 seconds), and I don't
know if other ARCHs have similar cases.

> 
> > Is this about the concern of possible TSC frequency calibration
> > issue, as the 40 ms per second drift between HPET and TSC? With 
> > b50db7095fe0 backported, we also have another patch to force TSC
> > calibration for those platforms which get the TSC freq directly
> > from CPUID or MSR and don't have such info in dmesg:
> >  "tsc: Refined TSC clocksource calibration: 2693.509 MHz" 
> > 
> > https://lore.kernel.org/lkml/20220509144110.9242-1-feng.tang@intel.com/
> > 
> > We did met tsc calibration issue due to some firmware issue, and
> > this can help to catch it. You can try it if you think it's relevant.
> 
> I am giving this a go, thank you!

Thanks for spending time testing it!

Thanks,
Feng