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

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2406 bytes --]

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.

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

> I mean that ieee80211_tx_status_irqsafe() is basicaly doing this:
> 
> ieee80211_tx_status_irqsafe()
> {
> 	skb_queue_tail(queue, skb);
> 	clear_unreliable_if_needed();
> 	ieee80211_queue_work(&local->hw, &local->tx_status_work);
> }
> 
> Would be easier to read that way too.

Ah. Hmm well, I dunno, splitting it up that much didn't seem useful to
me.


> > > > - * This function may not be called in IRQ context. Calls to this function
> > >                                          ^^^
> > > 
> > > > - * for a single hardware must be synchronized against each other. Calls
> > > > - * to this function and ieee80211_tx_status_irqsafe() may not be mixed
> > > > - * for a single hardware.
> > > > + * This function may not be called in hard or soft IRQ context. Calls
> > >                                          ^^^^ ^^ ^^^^ ^^^

> I was just highlighting the current kdoc for my first point on the e-mail.

Oh. Well it said "IRQ", but that was wrong, it should've said "hard
IRQ". Then again sometimes "IRQ" is used to refer to "hard interrupt"
only...

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]