From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751422AbdB0QWz (ORCPT ); Mon, 27 Feb 2017 11:22:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45780 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751046AbdB0QWy (ORCPT ); Mon, 27 Feb 2017 11:22:54 -0500 Subject: Re: tip.today - scheduler bam boom crash (cpu hotplug) To: Peter Zijlstra References: <1484811069.17751.14.camel@gmx.de> <20170119101924.GJ6485@twins.programming.kicks-ass.net> <20170119133633.GB6536@twins.programming.kicks-ass.net> <20170227124349.GU6515@twins.programming.kicks-ass.net> <3e7c34e2-757b-19e9-f7e5-9bfaadd79e5d@redhat.com> <20170227130440.GW6500@twins.programming.kicks-ass.net> <15367170-e1c0-8901-7e15-940d8d699c21@redhat.com> <20170227155954.GY6500@twins.programming.kicks-ass.net> Cc: Wanpeng Li , Mike Galbraith , LKML , Ingo Molnar , Thomas Gleixner , Borislav Petkov From: Paolo Bonzini Message-ID: <111f89b9-22e3-5a0f-3469-6f64092521ff@redhat.com> Date: Mon, 27 Feb 2017 17:11:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170227155954.GY6500@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 27 Feb 2017 16:11:37 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27/02/2017 16:59, Peter Zijlstra wrote: > OK, so if !KVM_FEATURE_CLOCKSOURCE_STABLE_BIT nothing is stable, but if > it is set, TSC might still not be stable, but kvm_clock_read() is. > >> However, if kvmclock is stable, we know that the sched clock is stable. > Right, so the problem is that we only ever want to allow marking > unstable -- once its found unstable, for whatever reason, we should > never allow going stable. The corollary of this proposition is that we > must start out assuming it will become stable. And to avoid actually > using unstable TSC we do a 3 state bringup: > > 1) sched_clock_running = 0, __stable_early = 1, __stable = 0 > 2) sched_clock_running = 1 (__stable is effective, iow, we run unstable) > 3) sched_clock_running = 2 (__stable <- __stable_early) > > 2) happens 'early' but is 'safe'. > 3) happens 'late', after we've brought up SMP and probed TSC > > Between there, we should have detected the most common TSC wreckage and > made sure to not then switch to 'stable' at 3. > > Now the problem appears to be that we assume sched_clock will use RDTSC > (native_sched_clock) while sched_clock is a paravirt op. > > Now, I've not yet figured out the ordering between when we set > pv_time_ops.sched_clock and when we do the 'normal' TSC init stuff. I think the ordering is fine: - pv_time_ops.sched_clock is set here: start_kernel (init/main.c line 509) setup_arch kvmclock_init kvm_sched_clock_init - TSC can be declared unstable only after this: start_kernel (init/main.c line 628) late_time_init tsc_init So by the time the tsc_cs_mark_unstable or mark_tsc_unstable can call clear_sched_clock_stable, pv_time_ops.sched_clock has been set. > But it appears to me, we should not be calling > clear_sched_clock_stable() on TSC bits when we don't end up using > native_sched_clock(). Yes, this makes sense. Paolo