Date: Tue, 01 Aug 2006 11:56:27 -0700 From: Sam Leffler <sam@errno.com> To: John Baldwin <jhb@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org>, Paolo Pisati <piso@freebsd.org> Subject: Re: PERFORCE change 102954 for review Message-ID: <44CFA3DB.2090801@errno.com> In-Reply-To: <200608011434.07440.jhb@freebsd.org> References: <200608011725.k71HP4ol019342@repoman.freebsd.org> <44CF928B.7020102@errno.com> <200608011434.07440.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?44CFA3DB.2090801>