On Sat, Nov 03, 2012 at 12:15:57PM +0100, Johannes Berg wrote:
> On Fri, 2012-11-02 at 11:49 +0100, Karl Beldan wrote:
>
> > > The patches also add flow control to stop incoming tx packets when the
> > > DMA ring is full. In practice I found that we will sometimes receive a
> > > single frame from mac80211 after stopping the queues, so some headroom
> > > is reserved when stopping the queues. I also reduced the number of tx
> > > descriptors per ring to 64 and fixed a bug that prevented having
> > > differing non-zero numbers of tx and rx descriptors for a given ring.
> > >
> > It is strange that you would get one frame after stopping the queues.
> > Apart from the iface up/down code which I did not look at, it seems the tx
> > codepaths for queues stop/wake are all properly protected by spin_lock_bhs.
> > You mention a possible race in your code comments .. are you referring to
> > mac80211 or the driver itself ?
>
> It's generally a race between mac80211 and the driver, if the driver
> uses ieee80211_stop_queue() outside the TX path, the TX path can be
> handing it a packet at the exact same time, and therefore there can be
> one packet (on each queue!) that goes to the driver during the call to
> stop_queue().
>
Yes, yet in this case, it seems to be another pb.
The driver performs ieee80211_stop_queue() inside the TX path exept for the
call within the tx status callback tasklet for ampdu retries - and the latter
should not trigger the pb since, as the comment puts it:
{
/*
* We shouldn't be out of space in the DMA
* ring here since we're reinserting a frame
* that was just pulled out.
*/
}
At first sight I would more likely doubt the txavail, though I haven't really
looked into the code, the potential race caught my attention within the diff
and I thought it might be useful to follow up.
Karl
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
