Re: [PATCH] ath5k: Invoke irqsafe version of ieee80211_tx_status() to avoid deadlock

[Bob Copeland <me@bobcopeland.com>]

On Wed, Aug 24, 2011 at 06:34:24PM -0700, Thomas Pedersen wrote:
> From: Javier Cardona <javier@cozybit.com>
> 
> This driver reports transmission status to the upper layer
> (ath5k_tx_frame_completed()) while holding the lock on the transmission
> queue (txq->lock).  Under failure conditions, the mesh stack will
> attempt to send PERR messages to the previous sender of the failed
> frame.  When that happens the driver will attempt to re-acquire the
> txq->lock lock causing a deadlock.  There are two possible fixes for
> this, (1) we could defer the transmission of the PERR frame until the
> lock is released or 

Of course, as a lazy driver writer I would like this one.  Esp. without
at least a comment in tx_status that it can call back into the driver.

> (2) release the lock before invoking ieee80211_tx_status().  

I was initially worried that we might race against draining buffers
that might also want to free the skb.  That is not the case (skb is
already removed from the structure) -- but there could potentially
be other races with drain since we're mucking with the list that
we're iterating over.  I'm least enthused with this approach.

> (see ath_tx_complete() in ath9k/xmit.c) as well as the rt2x00 and b43
> drivers.  The iwl driver, on the other hand, avoids this problem by
> invoking  ieee80211_tx_status_irqsafe() which effectively defers
> processing of transmission feedback status.  This last approach is the
> least intrusive is implemented here.

I guess it's ok.  The only real potential fallout would be worse rate
adaptation?  Perhaps needs a comment so no one fixes it later.

-- 
Bob Copeland %% www.bobcopeland.com