From owner-freebsd-current@FreeBSD.ORG Mon Jan 30 17:57:18 2012 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 783C5106566B for ; Mon, 30 Jan 2012 17:57:18 +0000 (UTC) (envelope-from freebsd@damnhippie.dyndns.org) Received: from qmta08.emeryville.ca.mail.comcast.net (qmta08.emeryville.ca.mail.comcast.net [76.96.30.80]) by mx1.freebsd.org (Postfix) with ESMTP id 5E0208FC0C for ; Mon, 30 Jan 2012 17:57:18 +0000 (UTC) Received: from omta23.emeryville.ca.mail.comcast.net ([76.96.30.90]) by qmta08.emeryville.ca.mail.comcast.net with comcast id Tsdh1i0011wfjNsA8tk82t; Mon, 30 Jan 2012 17:44:08 +0000 Received: from damnhippie.dyndns.org ([24.8.232.202]) by omta23.emeryville.ca.mail.comcast.net with comcast id Ttk61i00Z4NgCEG8jtk7Pn; Mon, 30 Jan 2012 17:44:07 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id q0UHi5KG019285; Mon, 30 Jan 2012 10:44:05 -0700 (MST) (envelope-from freebsd@damnhippie.dyndns.org) From: Ian Lepore To: Max Khon In-Reply-To: References: Content-Type: text/plain Date: Mon, 30 Jan 2012 10:44:05 -0700 Message-Id: <1327945445.1686.36.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.26.0 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: current@freebsd.org Subject: Re: FILTER_SCHEDULE_THREAD is not a bit-value X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Jan 2012 17:57:18 -0000 On Mon, 2012-01-30 at 22:50 +0600, Max Khon wrote: > Hello! > > sys/bus.h documents the following semantics for FILTER_SCHEDULE_THREAD: > > /** > * @brief Driver interrupt filter return values > * > * If a driver provides an interrupt filter routine it must return an > * integer consisting of oring together zero or more of the following > ^^^^^^^ > * flags: > * > * FILTER_STRAY - this device did not trigger the interrupt > * FILTER_HANDLED - the interrupt has been fully handled and can be EOId > * FILTER_SCHEDULE_THREAD - the threaded interrupt handler should be > * scheduled to execute > * > * If the driver does not provide a filter, then the interrupt code will > * act is if the filter had returned FILTER_SCHEDULE_THREAD. Note that it > * is illegal to specify any other flag with FILTER_STRAY and that it is > * illegal to not specify either of FILTER_HANDLED or FILTER_SCHEDULE_THREAD > * if FILTER_STRAY is not specified. > */ > #define FILTER_STRAY 0x01 > #define FILTER_HANDLED 0x02 > #define FILTER_SCHEDULE_THREAD 0x04 > > But actually FILTER_SCHEDULE_THREAD is not used as a bit-value (see > kern/kern_intr.c): > > if (!thread) { > if (ret == FILTER_SCHEDULE_THREAD) > thread = 1; > } > > There is at least one in-tree driver that could be broken because of > this (asmc(8), but I found the problem with some other out-of-tree > driver). > This should be "if (ret & FILTER_SCHEDULE_THREAD)" instead. Attached > patch fixes the problem. > > What do you think? > > Max I think returning (FILTER_HANDLED | FILTER_SCHEDULE_THREAD) makes no sense given the definition "the interrupt has been fully handled and can be EOId". If you EOI in the primary interrupt context and then schedule a threaded handler to run as well you're likely to need complex locking between the primary and threaded interrupt handlers and I was under the impression that's just the sort of thing the filter/threaded scheme was designed to avoid. In other words, the part about ORing together values seems to be staking out room for future growth, because the current set of flags and the words about how to use them imply that only one of the current set of values should be returned at once. On the other hand, the words are also self-contradictory, in that they say "oring together zero or more" but then later when saying which flags can be used together it's defined as erronious to return zero. -- Ian