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=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 A3DA8C2D0A3 for ; Fri, 30 Oct 2020 00:43:07 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2B97620691 for ; Fri, 30 Oct 2020 00:43:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="HJnnuzfd" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2B97620691 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=telegraphics.com.au Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:Message-ID:In-Reply-To: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=+hVtpibodDiwcNXV9/ojTMcIBYr+4mrrp+6MSiaa2pY=; b=HJnnuzfdTGgjeJLcn/SxkolmR baTMa+YYCU5HFtI9OJY5RikFQlPQ0trDiY0CBCd5kg1BvWRj+Nkz7JxAWmIvtw2SrO3WTffea/TzL Pl8kCRfPEXX1lF29sQUzadhiTliEccjTx1KxArHx2MlRT9qAnkN+b51qg7h7hbSOuh4Si5rKMBApz vj7NW3YeLHISGL76eRq87CgGLX/t5zCDdONNQlUWCOmfmSj2MCZspi+sW7puuTYshCzt5pAeVO5Cp Z7mRBiiNxzr1l7XCheght4wuBUSxpcgTAex4RpqMBGs1+EHTv3I+SDTF9ktf/wiCd5ybtoKWx+sjM LGFoGTmzg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kYIUC-0006qK-5I; Fri, 30 Oct 2020 00:41:44 +0000 Received: from kvm5.telegraphics.com.au ([98.124.60.144]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kYIU9-0006pQ-6A for linux-arm-kernel@lists.infradead.org; Fri, 30 Oct 2020 00:41:42 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by kvm5.telegraphics.com.au (Postfix) with ESMTP id 55B4C2A441; Thu, 29 Oct 2020 20:41:36 -0400 (EDT) Date: Fri, 30 Oct 2020 11:41:36 +1100 (AEDT) From: Finn Thain To: Arnd Bergmann Subject: Re: [RFC 13/13] m68k: mac: convert to generic clockevent In-Reply-To: Message-ID: References: <20201008154651.1901126-1-arnd@arndb.de> <20201008154651.1901126-14-arnd@arndb.de> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201029_204141_304792_152ED65C X-CRM114-Status: GOOD ( 46.98 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Sam Creasey , Fenghua Yu , Linus Walleij , Tony Luck , Thomas Gleixner , Parisc List , linux-ia64@vger.kernel.org, Stephen Boyd , Helge Deller , Daniel Lezcano , "linux-kernel@vger.kernel.org" , Russell King , "James E.J. Bottomley" , linux-m68k , Geert Uytterhoeven , Linux ARM , John Stultz , Philip Blundell , Greg Ungerer , Joshua Thompson Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 23 Oct 2020, Arnd Bergmann wrote: > On Sun, Oct 18, 2020 at 2:55 AM Finn Thain wrote: > > On Thu, 15 Oct 2020, Arnd Bergmann wrote: > > > On Thu, Oct 15, 2020 at 3:19 AM Finn Thain wrote: > > > > On Sat, 10 Oct 2020, Arnd Bergmann wrote: > > > > That configuration still produces the same 5 KiB of bloat. I see that > > kernel/time/Kconfig has this -- > > > > # Core internal switch. Selected by NO_HZ_COMMON / HIGH_RES_TIMERS. This is > > # only related to the tick functionality. Oneshot clockevent devices > > # are supported independent of this. > > config TICK_ONESHOT > > bool > > > > But my question was really about both kinds of dead code (oneshot > > device support and oneshot tick support). Anyway, after playing with > > the code for a bit I don't see any easy way to reduce the growth in > > text size. > > Did you look more deeply into where those 5KB are? Is this just the code > in kernel/time/{clockevents,tick-common}.c and the added platform > specific bits, or is there something more? What I did was to list the relevant functions using scripts/bloat-o-meter and tried stubbing out some code related to oneshot clockevent devices. I didn't find any low fruit and don't plan to pursue that without the help of LTO. > I suppose the sysfs interface and the clockevents_update_freq() logic > are not really needed on m68k, but it wouldn't make much sense to split > those out either. > > How does the 5KB bloat compare to the average bloat we get from one > release to the next? Geert has been collecting statistics for this. > Perhaps that 5 KB is justified by gaining the hrtimers feature... hard to say; it's never been available on these platforms. I can see the value in it though. > > > Yes, makes sense. I think the one remaining problem with the > > > periodic mode in this driver is that it can drop timer ticks when > > > interrupts are disabled for too long, while in oneshot mode there > > > may be a way to know how much time has passed since the last tick as > > > long as the counter does not overflow. > > > > Is there any benefit from adopting a oneshot tick (rather than > > periodic) if no clocksource is consulted when calculating the next > > interval? (I'm assuming NO_HZ is not in use, for reasons discussed > > below.) > > If the clocksource does not set CLOCK_SOURCE_IS_CONTINOUS, the kernel > will keep using periodic timers and not allow hrtimers. > IIUC, when HIGH_RES_TIMERS=y, the kernel will enable hrtimers only if the platform provides both a continuous clocksource device and a oneshot clockevent device. However, the "jiffies" clocksource does not set CLOCK_SOURCE_IS_CONTINOUS and neither does the one in arch/arm/mach-rpc/time.c. When HIGH_RES_TIMERS=n and NO_HZ_COMMON=n (which is presently the case for all platforms with GENERIC_CLOCKEVENTS=n) there's no use for a oneshot clockevent device, right? It seems likely that NO_HZ_COMMON=n because the clocksources on these platforms produce a periodic interrupt regardless (e.g. clocksource/i8253.c, arm/rpc, m68k platform timers etc.). Finally, HIGH_RES_TIMERS=n seems likely if the only clocksource is unreliable (e.g. because it loses time due to interrupt priorities). There may be a few of platforms in this category (Mac, Atari?). > > > I would agree that despite this oneshot mode is probably worse > > > overall for timekeeping if the register accesses introduce > > > systematic errors. > > > > > > > It probably has to be tried. But consulting a VIA clocksource on every > > tick would be expensive on this platform, so if that was the only way > > to avoid cumulative errors, I'd probably just stick with the periodic > > tick. > > I'm sure there is a tradeoff somewhere. Without hrtimers, some events > will take longer when they have to wait for the next tick, and using > NO_HZ_FULL can help help make things faster on some workloads. > Yes, such a tradeoff is discussed in drivers/iio/adc/ep93xx_adc.c. But OTOH, Documentation/timers/timers-howto.rst says, On slower systems, (embedded, OR perhaps a speed-stepped PC!) the overhead of setting up the hrtimers for usleep *may* not be worth it I guess it has to be tried. > ... > > The other 11 platforms in that category also have 'synthetic' > > clocksources derived from a timer reload interrupt. In 3 cases, the > > clocksource read method does not (or can not) check for a pending > > counter reload interrupt. For these also, I see no practical > > alternative to the approach you've taken in your RFC patch: > > > > arch/m68k/68000/timers.c > > arch/m68k/atari/time.c > > arch/m68k/coldfire/timers.c > > Agreed. It's possible there is a way, but I don't see one either. > For arch/m68k/68000/timers.c, I suppose we may be able to check for the TMR1 bit in the Interrupt Status Register at 0xFFFFF30C or the COMP bit in the Timer Status Register at 0xFFFFF60A. But testing that patch could be difficult. I expect that equivalent flags are available in Coldfire registers, making it possible to improve upon mcftmr_read_clk() and m68328_read_clk() if need be -- that is, if it turns out that the clocksource interrupt was subject to higher priority IRQs that would slow down the clocksource or defeat its monotonicity. The other difficulty is a lack of hardware timers. There's only one timer on MC68EZ328. On Atari, for now only Timer C is available though Michael has said that it would be possible to free up Timer D. Some Coldfire chips have only 2 timers and the second timer seems to be allocated to profiling. > > That leaves 8 platforms that have reliable clocksource devices which > > should be able to provide an accurate reading even in the presence of > > a dropped tick (due to drivers disabling interrupts for too long): > > > > arch/arm/mach-rpc/time.c > > arch/m68k/amiga/config.c > > arch/m68k/bvme6000/config.c > > arch/m68k/coldfire/sltimers.c > > arch/m68k/hp300/time.c > > arch/m68k/mac/via.c > > arch/m68k/mvme147/config.c > > arch/m68k/mvme16x/config.c > > > > But is there any reason to adopt a oneshot tick on any of these platforms, > > if NO_HZ won't eliminate the timer interrupt that's needed to run a > > 'synthetic' clocksource? > > I would expect that these could be changed to behave more like > drivers/clocksource/i8253.c, which does support a clocksource in oneshot > mode. > I think you meant to write, "... support a clockevent device in oneshot mode". I would agree. Since Macs do have a spare hardware timer, I will attempt to convert arch/m68k/mac/via.c to a oneshot clockevent, using your GENERIC_CLOCKEVENTS patch series as a basis, and see what effect that has in the NO_HZ_COMMON=n, HIGH_RES_TIMERS=y case. > Arnd > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel