Received-SPF: pass (google.com: domain of linux-kernel+bounces-11537-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Tue, 26 Dec 2023 22:16:33 +0800
From: Feng Tang <feng.tang@intel.com>
To: Jiri Wiesner <jwiesner@suse.de>
CC: <linux-kernel@vger.kernel.org>, John Stultz <jstultz@google.com>, "Thomas
 Gleixner" <tglx@linutronix.de>, Stephen Boyd <sboyd@kernel.org>, "Paul E.
 McKenney" <paulmck@kernel.org>
Subject: Re: [PATCH] clocksource: Use proportional clocksource skew threshold
Message-ID: <ZYrgQUTB3ayTtMqK@feng-clx>
References: <20231221160517.GA22919@incl>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231221160517.GA22919@incl>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?xcB3LJGP7nXx2bs+//cnVWX5iDxvq3RPvzbbLqCGkJ4qZMWErvc1N7sX5/Jh?=
 =?us-ascii?Q?A8La1TXXrrzQEEaM6Aj8qzRq5hzws6L5yU3gnIQa98bb/d5k/H63P22pSdVT?=
 =?us-ascii?Q?riT19zcPVJw+B58jpWTDm8ohJWDCv7uZIZqrUaCwuUAbJaRY5xO8aCzvO5Dc?=
 =?us-ascii?Q?R30BrlAvjS1B4aKWxa08KI4BjAo32MxPL6sMcJdBtbExnjJGKwZYLOInpuBX?=
 =?us-ascii?Q?Z2ZPUQIR5PhjtJXLlXAh1SxxmOeObl5hSpo3Ff5R+pzIXuW+P/3j29Gf9s//?=
 =?us-ascii?Q?tPzNs6kkY8Ue5L/HXcdmQ2TikI1uy33BO5aiL0+2AgtAWx732EVgScMbIfb4?=
 =?us-ascii?Q?OXom0qF0Vm+7SgU3SCWrlhOjhl6xWB3dwHlxZIZxmxZA3VBQX8AvbuuSWjiE?=
 =?us-ascii?Q?wsLFxbodqSAZ2umsG0xGSwLOVjZ7c5JHfKcgYUslycc/Y0+8mgxfMubFUS4O?=
 =?us-ascii?Q?DQNNnxKHsN1pw/kFGgV6K0hC79nxt8luxBX9f6nsSi7geLqAo3QKu5xLdhAY?=
 =?us-ascii?Q?nUKVCoHfucEvN3z1lJwNBcWm3Y+76AeKhI1OOa6CDevcMNjAmrcMNElnQfe+?=
 =?us-ascii?Q?0YejpraGnSkAIy8efaR8MK93WeFSgV8eQSwVy0YWg2IEdxaZvkrGsPkh92mo?=
 =?us-ascii?Q?t7pZqfcez0BNcH37Xr1m0OxqozIzcO7nf8JFuMi1yUMq+ZmNSrSyxXhzdctc?=
 =?us-ascii?Q?f93ggRJ1hI4gx+Rslbvv0mJbOxotXj1PRvRSpgNLvfkBBOiRRuBiDQXOWnJq?=
 =?us-ascii?Q?zaqkqsiaVWfuxlKqHHSyU+S1rL+5TEV1srgcLs9KF2f2PJ10NxPYz15uymEh?=
 =?us-ascii?Q?7GEYlLmlRSQXJoNhlcADBQRqwAv56cR+RtwWL64DFGZMyB7+EW5h6iqvokiG?=
 =?us-ascii?Q?S6H1OmrbfjWOpnziPY07Pkq9efRZxCz9bbbDOujvZ7MwxZDVIDg8Qvby8qwF?=
 =?us-ascii?Q?WozKIZWVyKhU6wr6L4Ie21Nn2bbei5pKO9m+osjacvVI4HnF7M/sRjIDu3nw?=
 =?us-ascii?Q?+RKq2BB5h7V+aMaTfxX8E8FuBObD9vlh+EeU8BNJH2pkEFfVwn/gnP6vU7LO?=
 =?us-ascii?Q?TIsr+ndM8tnGGbF8EZ25S/3YAT246TGr+CaMc0WkR20zx4rX/QrxjsN5VKHC?=
 =?us-ascii?Q?u9xu2rshqYwT4Wjiz8qFk4ComrGwoRSaykCmm6bP1E7CCrQq+U0WssmHTjY6?=
 =?us-ascii?Q?bGbLBKZebox8K0cFGoL7zoD6jFDYXGSzzbUApZi9kJIwMvCc8kppL+Dj8KMG?=
 =?us-ascii?Q?xlLODz3dHMybj29+6zmPhjeu3K7Nrov0WhRKddiD5KG9bsLjH2MOqnIhRtlu?=
 =?us-ascii?Q?k2YLe/Vrn6VtuEmSZ/ZM1zIEa2+B1YdNXqveZr2f4I6Nf8h3QlHGz/cmgw0H?=
 =?us-ascii?Q?C3Y/qiH1LOHbbyM7vZuflxZzepSbd/0NE7+hn27eM+IlKHzcbYu8I+WjISBY?=
 =?us-ascii?Q?lC+CnvuKu1HCNB/BTgttnLFcY1YiIRAwT6yZGuupErcTfoehqJE7HDhVmjtF?=
 =?us-ascii?Q?7euVtrecd4bjDh5KynvhVVgQhLA12G/OrKuprm7E5rQByit0WntPjSaHrKyO?=
 =?us-ascii?Q?ObD6DKW9nG3QD4y5vVmyfu6Bkvw77Zq57QkXVNPi?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4f881e01-4a13-4739-3ff3-08dc061e7e3e
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Dec 2023 14:25:21.8286
 (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: 5r2QGRbQThdL2p9+cK0kLMjtou+00TKfvesqNUDcXcC1iLJAc8iCaga9bCsmlCI6NsBuWMYz1q/t0cppfxzJEg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6318
X-OriginatorOrg: intel.com

On Thu, Dec 21, 2023 at 05:05:17PM +0100, Jiri Wiesner wrote:
> There have been reports of the watchdog marking clocksources unstable on
> machines 8 NUMA nodes:
> > clocksource: timekeeping watchdog on CPU373: Marking clocksource 'tsc' as unstable because the skew is too large:
> > clocksource:   'hpet' wd_nsec: 14523447520 wd_now: 5a749706 wd_last: 45adf1e0 mask: ffffffff
> > clocksource:   'tsc' cs_nsec: 14524115132 cs_now: 515ce2c5a96caa cs_last: 515cd9a9d83918 mask: ffffffffffffffff
> > clocksource:   'tsc' is current clocksource.
> > tsc: Marking TSC unstable due to clocksource watchdog
> > TSC found unstable after boot, most likely due to broken BIOS. Use 'tsc=unstable'.
> > sched_clock: Marking unstable (1950347883333462, 79649632569)<-(1950428279338308, -745776594)
> > clocksource: Checking clocksource tsc synchronization from CPU 400 to CPUs 0,46,52,54,138,208,392,397.
> > clocksource: Switched to clocksource hpet
> 
> The measured clocksource skew - the absolute difference between cs_nsec
> and wd_nsec - was 668 microseconds:
> > cs_nsec - wd_nsec = 14524115132 - 14523447520 = 667612
> 
> The kernel used 200 microseconds for the uncertainty_margin of both the
> clocksource and watchdog, resulting in a threshold of 400 microseconds.
> The discrepancy is that the measured clocksource skew was evaluated against
> a threshold suited for watchdog intervals of roughly WATCHDOG_INTERVAL,
> i.e. HZ >> 1. Both the cs_nsec and the wd_nsec value indicate that the
> actual watchdog interval was circa 14.5 seconds. Since the watchdog is
> executed in softirq context the expiration of the watchdog timer can get
> severely delayed on account of a ksoftirqd thread not getting to run in a
> timely manner. Surely, a system with such belated softirq execution is not
> working well and the scheduling issue should be looked into but the
> clocksource watchdog should be able to deal with it accordingly.

We've seen similar reports on LKML that the watchdog timer was delayed
for a very long time (some was 100+ seconds). As you said, the
scheduling issue should be addressed.

Meanwhile, instead of adding new complex logic to clocksource watchdog
code, can we just printk_once a warning message and skip the current
watchdog check if the duration is too long. ACPI_PM timer only has a
24 bit counter which will wrap around every 3~4 seconds, when the
duration is too long, like 14.5 seconds here, the check is already
meaningless.

Thanks,
Feng

> 
> To keep the limit imposed by NTP (500 microseconds of skew per second),
> the margins must be scaled so that the threshold value is proportional to
> the length of the actual watchdog interval. The solution in the patch
> utilizes left-shifting to approximate the division by
> WATCHDOG_INTERVAL * NSEC_PER_SEC / HZ, which leads to slighly narrower
> margins and a slightly lower threshold for longer watchdog intervals.
> 
> Fixes: 2e27e793e280 ("clocksource: Reduce clocksource-skew threshold")
> Signed-off-by: Jiri Wiesner <jwiesner@suse.de>
> ---
>  kernel/time/clocksource.c | 60 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 53 insertions(+), 7 deletions(-)
[...]