On 2015-03-18 21:07, Johannes Berg wrote: > On Wed, 2015-03-18 at 21:03 +0100, Felix Fietkau wrote: >> On 2015-03-18 20:41, Johannes Berg wrote: >> >> + * The driver is expected to release its own buffered frames and also call >> >> + * ieee80211_tx_dequeue() within that callback. >> > >> > Perhaps that should read >> > "The driver is expected to release its own buffered frames (if any) and >> > request the remaining dequeued frames by calling >> > ieee80211_tx_dequeue()." >> > >> > I'm not really sure it needs to be within that callback? I see no >> > particular reason for that. >> Releasing multiple packets works, even if there is only one packet >> buffered in the driver and the rest in the txq. It also keeps the code >> more consistent. > > Right. I still phrased that badly. I meant that the "also" should be > limited by the number of frames really needed, i.e. use driver-buffered > first and fill up with any mac80211-buffered by dequeuing. That's > probably obvious enough though. > > Anyway - not sure it needs to be in the callback? I think it makes things simpler this way. What would you prefer? >> If I don't lock here, one last dequeue call might still be running on >> another CPU. This would produce a theoretical race in accessing the >> sequence number, which the caller of this function reads before setting >> up the BA request. >> Dequeueing happens outside of the normal network stack tx context, so >> synchronize_net is not enough. > > Ah, makes sense, I didn't think of the seqno. Can you please put that in > a comment somewhere? :) Sure. - Felix -- 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
