From owner-svn-src-all@FreeBSD.ORG Tue Oct 19 13:16:08 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 6376A106567A; Tue, 19 Oct 2010 13:16:08 +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 1AFBC8FC15; Tue, 19 Oct 2010 13:16:08 +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 863D646B7F; Tue, 19 Oct 2010 09:16:07 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 0BE368A01D; Tue, 19 Oct 2010 09:16:06 -0400 (EDT) From: John Baldwin To: Alexander Motin Date: Tue, 19 Oct 2010 08:36:37 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201010171646.o9HGks2U038501@svn.freebsd.org> <20101018213055.GP1416@alchemy.franken.de> <4CBCBF25.8010902@FreeBSD.org> In-Reply-To: <4CBCBF25.8010902@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201010190836.37272.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Tue, 19 Oct 2010 09:16:06 -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, svn-src-all@freebsd.org, src-committers@freebsd.org, Marius Strobl 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: Tue, 19 Oct 2010 13:16:08 -0000 On Monday, October 18, 2010 5:41:57 pm Alexander Motin wrote: > Marius Strobl wrote: > > On Mon, Oct 18, 2010 at 05:05:24PM -0400, John Baldwin wrote: > >> On Monday, October 18, 2010 4:52:24 pm Marius Strobl wrote: > >>> 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(). > > > > Correct; but still you can't say that the MD interrupt code enters a > > critical section in general, neither is incrementing td_intr_nesting_level > > in intr_execute_handlers() protected by a critical section. > > > >> 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. > > > > Well, I agree that entering a critical section in the time events > > code would mean entering a nested critical section unnecessarily in > > case the clock driver uses a regular "fast" interrupt handler and > > that should be avoided. Still I don't think the event time front-end > > actually should need to worry about wrapping the callback in a > > critical section. > > Interrupt frame, required for hard-/stat-/profclock() operation is > stored in curthread. So critical section is effectively mandatory there > now. Correct td_intr_nesting_level value is also important for proper > interrupt threads scheduling - one more reason to have critical section > there. It is indeed strange that td_intr_nesting_level in > intr_event_handle() is not covered by critical section, but probably it > should. I don't see why the interrupt frame logic requires bumping td_intr_nesting_level. I agree with bde@ that in general td_intr_nesting_level is useless (it is always 0 for most code and always 1 for filters and the ithread code that schedules an ithread). It does not need any protection for interrupts since the CPU has already disabled nesting interrupts. The critical_enter()'s sole purpose is to avoid a kind of priority inversion to ensure that the very high priority interrupt code (filters, etc.) is not preempted by an ithread. As far as the interrupt frame logic, the logic should have no need of td_intr_nesting_level. An interrupt should just save the current value of td_intr_frame before setting it and restore the saved value afterwards. No need for dealing with td_intr_nesting_level at all. -- John Baldwin