[rt2x00-users] [RFC] rt2x00: Use tasklets for rt2x00usb

Helmut Schaa helmut.schaa at googlemail.com
Thu Jul 22 20:56:21 AEST 2010


Am Donnerstag 22 Juli 2010 schrieb Ivo Van Doorn:
> On Thu, Jul 22, 2010 at 12:26 PM, Helmut Schaa
> <helmut.schaa at googlemail.com> wrote:
> > Am Donnerstag 22 Juli 2010 schrieb Ivo Van Doorn:
> >> On Thu, Jul 22, 2010 at 12:11 PM, Helmut Schaa
> >> <helmut.schaa at googlemail.com> wrote:
> >> > Am Donnerstag 22 Juli 2010 schrieb Ivo Van Doorn:
> >> >> On Thu, Jul 22, 2010 at 9:19 AM, Helmut Schaa
> >> >> <helmut.schaa at googlemail.com> wrote:
> >> >> > Am Donnerstag 22 Juli 2010 schrieb Ivo Van Doorn:
> >> >> >> On Thu, Jul 22, 2010 at 8:53 AM, Helmut Schaa
> >> >> >> <helmut.schaa at googlemail.com> wrote:
> >> >> >> > Am Donnerstag 22 Juli 2010 schrieb Mark Asselstine:
> >> >> >> >> On Wed, Jul 21, 2010 at 6:32 PM, Ivo van Doorn <ivdoorn at gmail.com> wrote:
> >> >> >> >> > This updates the rt2x00usb driver to use tasklets for handling interrupts.
> >> >> >> >> > This simplifies the code in rt2x00lib since it no longer needs to check if the
> >> >> >> >> > device is USB or PCI to decide which mac80211 function should be used.
> >> >> >> >> >
> >> >> >> >> > This patch is work in progress, but I would to hear if there are any comments
> >> >> >> >> > on the approach taken.
> >> >> >> >> >
> >> >> >> >> > Signed-off-by: Ivo van Doorn <IvDoorn at gmail.com>
> >> >> >> >> > ---
> >> >> >> >>
> >> >> >> >> Ivo,  I know it has been far too long since I have done any work on
> >> >> >> >> this project and I am far from an expert so feel free hit the delete
> >> >> >> >> key... that being said. Tasklets are used less and less in the kernel
> >> >> >> >> as there are now other mechanisms which offer the same flexibility.
> >> >> >> >> Seeing this trend makes me feel rt2x00 is moving in the opposite
> >> >> >> >> direction. Have you looked at threaded interrupts (which are now in
> >> >> >> >> mainline) as an alternative? If I can get my development environment
> >> >> >> >> setup quickly I will try to do better then these few words and
> >> >> >> >> actually attempt an implementation if you think it is worth it.
> >> >> >> >
> >> >> >> > I agree. We switched all rt2x00 PCI drivers to use threaded interrupts
> >> >> >> > already. However, since the USB versions don't have something like an
> >> >> >> > interrupt handler I don't know how we could move the rx/tx handling
> >> >> >> > into process context without introducing delays (for example when using a
> >> >> >> > workqueue) and concurrency.
> >> >> >>
> >> >> >> The reason why I made the switch was in preparation for a different patch,
> >> >> >> which actually needs a short delay between the URB callback function and
> >> >> >> the processing of the data. And a side-benefit is that it manages to clean some
> >> >> >> stuff up in the TX/RX processing which is also nice. :)
> >> >> >
> >> >> > Ok, understood. I don't know what you're working on right now ;)
> >> >>
> >> >> Oh, lets say that I am working on one of your ideas regarding rt2800usb
> >> >> and rate control mechanisms :P
> >> >
> >> > Cool!
> >> >
> >> >> > but the
> >> >> > tasklet approach should be fine (in regard to the mac80211 interaction at
> >> >> > least).
> >> >>
> >> >> Well I just did some testing and rt2800usb needs to use a mutex, so
> >> >> tasklets aren't the right tool (Doh!). I'll convert it to the workqueue :)
> >> >
> >> > Hmm, hopefully the workqueue doesn't delay too much.
> >>
> >> Doesnt really matter I think. There must be a delay because when the
> >> URB callback
> >> function is called, the packet isn't transmitted yet. So the delay is
> >> mandatory to
> >> make sure the TX status is set in the registers. I think the danger
> >> with delays with
> >> this feature is a too short delay rather then a too long delay.
> >
> > Right. I thought a bit further already because I'm working on a "stuck queue
> > problem" in rt2800pci right now that might touch similar parts in the code ;)
> >
> > The problem seems to happen under high load and the tx queue has 24 entries
> > while the TX STATUS register has only 16 -> not all entries in the queue get
> > cleaned up and after a while the queue is full of entries that won't be freed
> > anymore.
> >
> > That's why I'm trying to split up txdone reporting from the actual tx status
> > reporting. Reporing txdone to rt2x00lib means that the according entry in the
> > tx ring can be freed while the txstatus means the status should be sent to
> > mac80211. This makes sense since rt61pci and rt2800pci have different interrupts
> > for DMA_DONE and TX_STATUS ...
> >
> > Maybe the same approach would be useful for your issue as well? Free the tx
> > entry already in the usb txdone callback while the txstatus reporting is done
> > from a workqueue?
> 
> Interesting point, but this wouldn't work for rt2x00usb. The problem is that
> in the URB handler you can't read from the register due to the mutex which
> is grabbed.

Ok, but what I meant is: you can simply free the according tx entry without
reporting the status and report the status "later" from the workqueue? I mean
the tx entry cleaning is already done from the usb callback, right?

> Can;t you fix the PCI driver by listening to the INT_SOURCE_CSR_TX_FIFO_STATUS
> interrupt which triggers an extra call to txdone?

At the moment, we only listen to INT_SOURCE_CSR_TX_FIFO_STATUS and not to
the DMA_DONE interrups at all. But the correct way is to free the tx entries
from the DMA_DONE interrupts and report the status once the
INT_SOURCE_CSR_TX_FIFO_STATUS interrupt fires.

But I don't know if that works out or not, needs some more investigation first.

Helmut




More information about the users mailing list