From owner-svn-src-head@FreeBSD.ORG Mon Oct 18 21:05:30 2010 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id ECC681065673; Mon, 18 Oct 2010 21:05:29 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id A33D48FC16; Mon, 18 Oct 2010 21:05:29 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 1DD7A46B17; Mon, 18 Oct 2010 17:05:29 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 0840B8A009; Mon, 18 Oct 2010 17:05:28 -0400 (EDT) From: John Baldwin To: Marius Strobl Date: Mon, 18 Oct 2010 17:05:24 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201010171646.o9HGks2U038501@svn.freebsd.org> <4CBCADDD.5070109@FreeBSD.org> <20101018205224.GO1416@alchemy.franken.de> In-Reply-To: <20101018205224.GO1416@alchemy.franken.de> MIME-Version: 1.0 Message-Id: <201010181705.24879.jhb@freebsd.org> Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Mon, 18 Oct 2010 17:05:28 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: svn-src-head@freebsd.org, Alexander Motin , src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r213985 - head/sys/sparc64/sparc64 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Oct 2010 21:05:30 -0000 On Monday, October 18, 2010 4:52:24 pm Marius Strobl wrote: > On Mon, Oct 18, 2010 at 11:28:13PM +0300, Alexander Motin wrote: > > Marius Strobl wrote: > > > On Mon, Oct 18, 2010 at 10:03:12AM -0400, John Baldwin wrote: > > >> On Sunday, October 17, 2010 12:46:54 pm Marius Strobl wrote: > > >>> Author: marius > > >>> Date: Sun Oct 17 16:46:54 2010 > > >>> New Revision: 213985 > > >>> URL: http://svn.freebsd.org/changeset/base/213985 > > >>> > > >>> Log: > > >>> - In oneshot-mode it doesn't make sense to try to compensate the clock > > >>> drift in order to achieve a more stable clock as the tick intervals may > > >>> vary in the first place. In fact I haven't seen this code kick in when > > >>> in oneshot-mode so just skip it in that case. > > >>> - There's no need to explicitly stop the (S)TICK counter in oneshot-mode > > >>> with every tick as it just won't trigger again with the (S)TICK compare > > >>> register set to a value in the past (with a wrap-around once every ~195 > > >>> years of uptime at 1.5 GHz this isn't something we have to worry about > > >>> in practice). > > >>> - Given that we'll disable interrupts completely anyway there's no > > >>> need to enter critical sections. > > >> This last is not entirely true. The purpose of the critical section is to > > >> prevent the kernel from preempting to the softclock swi thread until all of > > >> the hardclock handler has finished execution. Thus, places that actually > > >> actually call hardclock() should probably still be wrapped in a critical > > >> section. > > > > > > It's currently unclear to me how on architectures converted to the > > > event timer world order hardclock() is called eventually but in any case > > > shouldn't it be the responsibility of the code actually calling it (or > > > the equivalent code) to wrap it in a critical section instead then? After > > > all the MD part just enrolls in calling _something_ in one-shot and/or > > > periodic mode without knowing what it actually calls (and IMO it also > > > should no longer need to). In handleevents() of kern_clocksource.c > > > hardclock_anycpu() is called so i think that is what actually needs to > > > be wrapped in a critical section. > > > > At this time on most (all?) platforms critical section is grabbed by MD > > interrupt code. It is important to be there, as soon as there touched > > td_intr_nesting_level and td_intr_frame fields of curthread. We can't > > allow thread migration until all counted interrupt handlers complete. > > > > AFAICT this is not true; intr_event_handle() in sys/kern/kern_intr.c > is what enters a critical section and f.e. on amd64 I don't see where > anywhere in the path from ISR_VEC() to intr_execute_handlers() > calling intr_event_handle() a critical section would be entered, > which also means that in intr_execute_handlers() td_intr_nesting_level > is incremented outside of a critical section. Not all of the clock interrupts use intr_event_handle(). The local APIC timer uses its own interrupt entry point on x86 for example and uses an explicit critical section as a result. I suspect the sparc64 tick interrupt is closer to the local APIC timer case and doesn't use intr_event_handle(). The fact that some clock interrupts do use intr_event_handle() (e.g. the atrtc driver on x86 now) does indicate that the low-level interrupt code probably does not belong in the time events code but in the caller. -- John Baldwin