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:
signature.asc
Description: This is a digitally signed message part
