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? > Now that I'm thinking about this some more, it might even make sense to > skip the sta PS queue for txq-enabled drivers. That would allow all sta > data frames to either go through driver scheduling or > release_buffered_frames. Well, it's slightly more complicated due to the filtered queue. Not sure you want to 'pollute' the TXQ abstraction with that? > 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? :) johannes -- 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
