From owner-svn-src-all@FreeBSD.ORG Mon Oct 18 20:52:26 2010 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BBC5D106564A; Mon, 18 Oct 2010 20:52:26 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 1D3AC8FC17; Mon, 18 Oct 2010 20:52:25 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.4/8.14.4/ALCHEMY.FRANKEN.DE) with ESMTP id o9IKqOjs035128; Mon, 18 Oct 2010 22:52:25 +0200 (CEST) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.4/8.14.4/Submit) id o9IKqOuH035127; Mon, 18 Oct 2010 22:52:24 +0200 (CEST) (envelope-from marius) Date: Mon, 18 Oct 2010 22:52:24 +0200 From: Marius Strobl To: Alexander Motin Message-ID: <20101018205224.GO1416@alchemy.franken.de> References: <201010171646.o9HGks2U038501@svn.freebsd.org> <201010181003.13045.jhb@freebsd.org> <20101018190615.GL1416@alchemy.franken.de> <4CBCADDD.5070109@FreeBSD.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4CBCADDD.5070109@FreeBSD.org> User-Agent: Mutt/1.4.2.3i Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, John Baldwin Subject: Re: svn commit: r213985 - head/sys/sparc64/sparc64 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Oct 2010 20:52:26 -0000 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. Marius