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:   Fri, 2 Jun 2023 11:00:55 -0700
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     Feng Tang <feng.tang@intel.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@intel.com>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>, x86@kernel.org,
        linux-kernel@vger.kernel.org, rui.zhang@intel.com,
        tim.c.chen@intel.com, Xiongfeng Wang <wangxiongfeng2@huawei.com>,
        liaoyu15@huawei.com
Subject: Re: [PATCH v1 2/2] x86/tsc: Extend watchdog check exemption to
 4-Sockets platform
Message-ID: <06df410c-2177-4671-832f-339cff05b1d9@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <20221021062131.1826810-1-feng.tang@intel.com>
 <20221021062131.1826810-2-feng.tang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221021062131.1826810-2-feng.tang@intel.com>
Precedence: bulk

On Fri, Oct 21, 2022 at 02:21:31PM +0800, Feng Tang wrote:
> There is report again that the tsc clocksource on a 4 sockets x86
> Skylake server was wrongly judged as 'unstable' by 'jiffies' watchdog,
> and disabled [1].
> 
> Commit b50db7095fe0 ("x86/tsc: Disable clocksource watchdog for TSC
> on qualified platorms") was introduce to deal with these false
> alarms of tsc unstable issues, covering qualified platforms for 2
> sockets or smaller ones.
> 
> Extend the exemption to 4 sockets to fix the issue.
> 
> We also got similar reports on 8 sockets platform from internal test,
> but as Peter pointed out, there was tsc sync issues for 8-sockets
> platform, and it'd better be handled architecture by architecture,
> instead of directly changing the threshold to 8 here.
> 
> Rui also proposed another way to disable 'jiffies' as clocksource
> watchdog [2], which can also solve this specific problem in an
> architecture independent way, with one limitation that there are
> also some tsc false alarms which were reported by other hardware
> watchdogs like HPET/PMTIMER, while 'jiffies' watchdog is mostly
> used in kernel boot phase.
> 
> [1]. https://lore.kernel.org/all/9d3bf570-3108-0336-9c52-9bee15767d29@huawei.com/
> [2]. https://lore.kernel.org/all/bd5b97f89ab2887543fc262348d1c7cafcaae536.camel@intel.com/
> 
> Reported-by: Yu Liao <liaoyu15@huawei.com>
> Signed-off-by: Feng Tang <feng.tang@intel.com>

We have a number of four-socket systems whose TSCs seem to be reliable.
We do see issues where high memory load forces the TSC to be marked
unstable, but that is because those systems are using an older kernel.

If the TSCs do start to misbehave, I will of course let you all know.
But in the meantime:

Reviewed-by: Paul E. McKenney <paulmck@kernel.org>

The previous patch that changes the definition of "socket" I have no
opinion on.  I must let you guys work that out.  However, I do note that
this patch can be rebased so as to no longer depend on that patch.

						Thanx, Paul

> ---
>  arch/x86/kernel/tsc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 178448ef00c7..356f06287034 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1400,7 +1400,7 @@ static int __init init_tsc_clocksource(void)
>  	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
>  	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
>  	    boot_cpu_has(X86_FEATURE_TSC_ADJUST) &&
> -	    logical_packages <= 2)
> +	    logical_packages <= 4)
>  		clocksource_tsc.flags &= ~CLOCK_SOURCE_MUST_VERIFY;
>  
>  	/*
> -- 
> 2.34.1
>