From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 638D4C43387 for ; Fri, 4 Jan 2019 15:39:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 32C1A21872 for ; Fri, 4 Jan 2019 15:39:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727352AbfADPjy (ORCPT ); Fri, 4 Jan 2019 10:39:54 -0500 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:44702 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727038AbfADPjx (ORCPT ); Fri, 4 Jan 2019 10:39:53 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1F92715AD; Fri, 4 Jan 2019 07:39:48 -0800 (PST) Received: from big-swifty.misterjones.org (big-swifty.cambridge.arm.com [10.1.36.144]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 025FA3F575; Fri, 4 Jan 2019 07:39:36 -0800 (PST) Date: Fri, 04 Jan 2019 15:39:22 +0000 Message-ID: <86bm4wbff9.wl-marc.zyngier@arm.com> From: Marc Zyngier To: Pavel Tatashin Cc: catalin.marinas@arm.com, Will Deacon , Andrew Morton , rppt@linux.vnet.ibm.com, Michal Hocko , Ard Biesheuvel , andrew.murray@arm.com, james.morse@arm.com, sboyd@kernel.org, linux-arm-kernel@lists.infradead.org, LKML Subject: Re: [PATCH v3 3/3] arm64: Early boot time stamps In-Reply-To: References: <20181226164509.22916-1-pasha.tatashin@soleen.com> <20181226164509.22916-4-pasha.tatashin@soleen.com> <9fe670d4-d799-c05f-297e-437eda1d2072@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/25.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Organization: ARM Ltd MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 03 Jan 2019 19:58:25 +0000, Pavel Tatashin wrote: > > > I still think this approach is flawed. You provide the kernel with a > > potentially broken sched_clock that may jump back and forth until the > > workaround kicks in. Nobody expects this. > > > > Instead, I'd suggest you allow for a something other than local_clock() > > to be used for the time stamping until a properly working sched_clock > > gets registered. > > > > This way, you'll only impact the timestamps when running on a broken system. > > I think, given that on other platforms sched_clock() is already used > early, it is not a good idea to invent a different clock just for time > stamps. Square pegs vs round holes. Mimicking other architectures isn't always the right thing to do when faced with a different problem. We put a lot of effort in working around timer errata for a good reason, and feeding the rest of the system bogus timing information doesn't sound great. > We could limit arm64 approach only for chips where cntvct_el0 is > working: i.e. frequency is known, and the clock is stable, meaning > cannot go backward. Perhaps we would start early clock a little later, > but at least it will be available for the sane chips. The only > question, where during boot time this is known. How do you propose we do that? Defective timers can be a property of the implementation, of the integration, or both. In any case, it requires firmware support (DT, ACPI). All that is only available quite late, and moving it earlier is not easily doable. > Another approach is to modify sched_clock() in > kernel/time/sched_clock.c to never return backward value during boot. > > 1. Rename current implementation of sched_clock() to sched_clock_raw() > 2. New sched_clock() would look like this: > > u64 sched_clock(void) > { > if (static_branch(early_unstable_clock)) > return sched_clock_unstable(); > else > return sched_clock_raw(); > } > > 3. sched_clock_unstable() would look like this: > > u64 sched_clock_unstable(void) > { > again: > static u64 old_clock; > u64 new_clock = sched_clock_raw(); > static u64 old_clock_read = READ_ONCE(old_clock); > /* It is ok if time does not progress, but don't allow to go backward */ > if (new_clock < old_clock_read) > return old_clock_read; > /* update the old_clock value */ > if (cmpxchg64(&old_clock, old_clock_read, new_clock) != old_clock_read) > goto again; > return new_clock; > } You now have an "unstable" clock that is only allowed to move forward, until you switch to the real one. And at handover time, anything can happen. It is one thing to allow for the time stamping to be imprecise. But imposing the same behaviour on other parts of the kernel that have so far relied on a strictly monotonic sched_clock feels like a bad idea. What I'm proposing is that we allow architectures to override the hard tie between local_clock/sched_clock and kernel log time stamping, with the default being of course what we have today. This gives a clean separation between the two when the architecture needs to delay the availability of sched_clock until implementation requirements are discovered. It also keep sched_clock simple and efficient. To illustrate what I'm trying to argue for, I've pushed out a couple of proof of concept patches here[1]. I've briefly tested them in a guest, and things seem to work OK. Thanks, M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=arm64/tsclock -- Jazz is not dead, it just smell funny.