Re: [PATCH v2] mac80211: expose txq queue depth and size to drivers
On Wed, 2016-01-27 at 15:26 +0100, Michal Kazior wrote: > This will allow drivers to make more educated > decisions whether to defer transmission or not. > > Relying on wake_tx_queue() call count implicitly > was not possible because it could be called > without queued frame count actually changing on > software tx aggregation start/stop code paths. > > It was also not possible to know how long > byte-wise queue was without dequeueing. > Applied. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mac80211: expose txq queue depth and size to drivers
On Wed, 2016-01-27 at 15:33 +0100, Michal Kazior wrote: > On 27 January 2016 at 15:26, Johannes Berg > wrote: > > On Wed, 2016-01-27 at 15:26 +0100, Michal Kazior wrote: > > > > > > @@ -1294,6 +1298,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct > > > ieee80211_hw *hw, > > > if (!skb) > > > goto out; > > > > > > + txqi->byte_cnt -= skb->len; > > > + > > > atomic_dec(&sdata->txqs_len[ac]); > > > > > This *looks* a bit worrying - you have an atomic dec for the # of > > packets and a non-atomic one for the bytes. > > > > You probably thought about it and I guess it's fine, but can you > > explain it? > > The atomic was used because it accesses per-vif counters. You can't > use txqi->queue.lock for that. > Ah. I completely missed that distinction, thanks. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mac80211: expose txq queue depth and size to drivers
On 27 January 2016 at 15:26, Johannes Berg wrote: > On Wed, 2016-01-27 at 15:26 +0100, Michal Kazior wrote: >> >> @@ -1294,6 +1298,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct >> ieee80211_hw *hw, >> if (!skb) >> goto out; >> >> + txqi->byte_cnt -= skb->len; >> + >> atomic_dec(&sdata->txqs_len[ac]); >> > This *looks* a bit worrying - you have an atomic dec for the # of > packets and a non-atomic one for the bytes. > > You probably thought about it and I guess it's fine, but can you > explain it? The atomic was used because it accesses per-vif counters. You can't use txqi->queue.lock for that. On the other hand byte_cnt is per txqi and it was very easy to make use of the txqi->queue.lock (which was already acquired in both drv_tx and tx_dequeue functions). Using atomic* for byte_cnt would be wasteful a bit. Michał -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mac80211: expose txq queue depth and size to drivers
On Wed, 2016-01-27 at 15:26 +0100, Michal Kazior wrote: > > @@ -1294,6 +1298,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct > ieee80211_hw *hw, > if (!skb) > goto out; > > + txqi->byte_cnt -= skb->len; > + > atomic_dec(&sdata->txqs_len[ac]); > This *looks* a bit worrying - you have an atomic dec for the # of packets and a non-atomic one for the bytes. You probably thought about it and I guess it's fine, but can you explain it? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
