From owner-p4-projects@FreeBSD.ORG Tue Aug 1 18:56:37 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 4D60A16A4E8; Tue, 1 Aug 2006 18:56:37 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0F01F16A4DE; Tue, 1 Aug 2006 18:56:37 +0000 (UTC) (envelope-from sam@errno.com) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7E5FD43D4C; Tue, 1 Aug 2006 18:56:29 +0000 (GMT) (envelope-from sam@errno.com) Received: from [10.0.0.248] (trouble.errno.com [10.0.0.248]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id k71IuRQJ024111 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 1 Aug 2006 11:56:28 -0700 (PDT) (envelope-from sam@errno.com) Message-ID: <44CFA3DB.2090801@errno.com> Date: Tue, 01 Aug 2006 11:56:27 -0700 From: Sam Leffler User-Agent: Thunderbird 1.5.0.4 (X11/20060724) MIME-Version: 1.0 To: John Baldwin References: <200608011725.k71HP4ol019342@repoman.freebsd.org> <44CF928B.7020102@errno.com> <200608011434.07440.jhb@freebsd.org> In-Reply-To: <200608011434.07440.jhb@freebsd.org> X-Enigmail-Version: 0.94.0.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Perforce Change Reviews , Paolo Pisati Subject: Re: PERFORCE change 102954 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Aug 2006 18:56:37 -0000 John Baldwin wrote: > On Tuesday 01 August 2006 13:42, Sam Leffler wrote: >> Paolo Pisati wrote: >>> http://perforce.freebsd.org/chv.cgi?CH=102954 >>> >>> Change 102954 by piso@piso_newluxor on 2006/08/01 17:24:16 >>> >>> Convert ath to use a filter+ithread handler: >>> >>> use a spinlock (inside softc) to guard against >>> races when accessing sc_status or the interrupt registers, >>> and axe all the taskqueue jobs from ath_intr(). >> I highly doubt this does the right thing and the spinlock is almost >> certain to be the wrong thing to do here. We should probably talk >> privately about how to restructure ath to use your stuff but I expected >> a very different approach. > > I think it shouldn't actually need a lot of restructuring at all. The > existing ath_intr() function should be the filter, but instead of creating a > task for the higher level handle, it just returns a flag to ask the interrupt > code to schedule it. This only handles 1 handler though, so if you kick off > multiple tasks you might need to do some tweaking there (either do the most > common one as the handler and still use taskqueue for the others or perhaps > have the handler be a little "fatter"). This does seem like a lot more code > churn than is necessary. > Basically yes, the stuff run in the private taskq thread should run in the deferred interrupt context. The only issue is how to handle beacon processing (and in the future UAPSD processing). I'm not familiar with what's allowed to run in a filter routine but deferring the beacon frame generation is likely not going to work and the work done to post a beacon frame may do calls that are not permitted. FWIW the model used by ath is to defer "slow processing" to taskq-like context and leverage the single-threaded-ness of the taskq thread to avoid contention. This has worked ok but adds overhead and I've considered doing a total rewrite. The existing scheme also means you cannot implement polling w/o major changes. Sam