Re: [RFC] mac80211: move TX status processing to process context

["Luis R. Rodriguez" <lrodriguez@atheros.com>]

On Wed, Nov 18, 2009 at 12:34:39PM -0800, Johannes Berg wrote:
> On Wed, 2009-11-18 at 12:23 -0800, Luis R. Rodriguez wrote:
> 
> > > > > +	/* if we got over the limit with this frame, try to make room */
> > > > > +	if (num > IEEE80211_TX_STATUS_QUEUE_LIMIT &&
> > > > > +	    (skb = skb_dequeue(&local->tx_status_unreliable))) {
> > > > >  		dev_kfree_skb_irq(skb);
> > > > 
> > > > Note before there was a while loop and now just a if branch. Why can we
> > > > get away with freeing now just one buffer? It would be nice to see
> > > > this explained in the commit log entry as it is not obvious to me.
> > > 
> > > Ah yes, I meant to explain that. We only use the status queues for TX
> > > status now and not for RX too as before, so before it could actually
> > > happen that we could free more than one off the unreliable queue, while
> > > now it really can only be one.
> > 
> > Ah, thanks, so, that skb_queue can go and we can just have one
> > unreliable sk_buff ?
> 
> No, you misunderstood. Because I'm separating the queue for the tasklet
> (tasklet_queue) and for TX status (tx_status/tx_status_unreliable) we
> can only possibly free one since we only added one -- the count cannot
> increase over that. The loop would only loop once at most.

OK, but can't you still have a driver spam mac80211 with a lot of
ieee80211_tx_status_irqsafe() calls in soft irq context with the final
skb requiring the tx complete, in that case the queue *will* get stuffed
and you could potentially free more if so desired.

Also, if our goal is to just avoid adding the skb if it does not require
a tx complete and our queue size is too large

	if (!(info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) &&
	    num + 1 > IEEE80211_TX_STATUS_QUEUE_LIMIT)
		dev_kfree_skb_irq(skb);
	else
		skb_queue_tail(&local->skb_queue)

> However ... right now we never use _any_ unreliable at all, but I
> suspect we will want to change that again at some point.

Just curious -- what would be a use case for that?

  Luis