[rt2x00-users] rt2x00lib_txdone() vs. ieee80211_tx_status_ni()

[Johannes Stezenbach]

Hi Helmut,

On Mon, Nov 29, 2010 at 06:32:25PM +0100, Helmut Schaa wrote:
> Am Montag 29 November 2010 schrieb Johannes Stezenbach:
> > (moving this thread from linux-wireless list to rt2x00 list)
> 
> Hey, just wanted to answer to your mail on linux-wireless :P

Oops, sorry.  I just thought discussion of rt2x00 internals is
off-topic on linux-wireless, too much traffic there already.

> > On Mon, Nov 29, 2010 at 05:13:16PM +0100, Johannes Stezenbach wrote:
> > > ieee80211_tx_status() documentation says "This function may not be
> > > called in IRQ context", and it is called by rt2800usb
> > > from a workqueue context.  However, ieee80211_tx_status() is
> > > meant to be called from tasklets and thus uses netif_rx().
> > > Add a new ieee80211_tx_status_ni() which does the same
> > > thing but is safe to be called from process context.
> > > 
> > > Using ieee80211_tx_status_ni() for rt2800usb fixes the
> > > "NOHZ: local_softirq_pending 08" messages I've been getting.
> > ...
> > > Note: the patch is now incomplete, rt2x00lib_txdone() is used by
> > > several drivers, rt2800usb calls it from a workqueue while
> > > e.g. while rt2800pci calls it from a tasklet
> > > (rt2800pci_txstatus_tasklet).  I need to sort this out with
> > > the rt2x00 developers.
> 
> Hmm, it should be fine to call local_bh_disable from within a tasklet, no?
> It's not necessary but shouldn't cause any trouble and only adds a small
> overhead.

Hm, I guess performance is the point in distinguishing between
ieee80211_tx_status() and ieee80211_tx_status_ni(), so I guess
we should take the hint and do the right thing, too.

BTW, thinking about it, rt2x00usb_work_txdone() can process more
than one packet at once, so I wonder about the performance
implications of using ieee80211_tx_status_ni(), i.e. several
do_softirq() calls each processing one packet.  Would it
not be better to do the local_bh_disable/enable() in rt2x00usb_work_txdone()
and then let it call ieee80211_tx_status()?

And probably similarly for rt2x00usb_work_rxdone() and ieee80211_rx()?


Thanks,
Johannes