[PATCH 1/2] ath9k: try more than one tid when scheduling a new aggregate

[PATCH 1/2] ath9k: try more than one tid when scheduling a new aggregate

[Felix Fietkau]

Sometimes the first TID in the first AC's list is not available for forming
a new aggregate (the BAW might not allow it), however other TIDs may have
data available for sending.
Prevent a slowdown of other TIDs by going through multiple entries until
we've either hit the last one or enough AMPDUs are pending in the hardware
queue.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/xmit.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index ab4f7b4..fffd13d 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1224,12 +1224,14 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
 void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
 {
 	struct ath_atx_ac *ac;
-	struct ath_atx_tid *tid;
+	struct ath_atx_tid *tid, *last;
 
-	if (list_empty(&txq->axq_acq))
+	if (list_empty(&txq->axq_acq) ||
+	    txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH)
 		return;
 
 	ac = list_first_entry(&txq->axq_acq, struct ath_atx_ac, list);
+	last = list_entry(ac->tid_q.prev, struct ath_atx_tid, list);
 	list_del(&ac->list);
 	ac->sched = false;
 
@@ -1253,7 +1255,8 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
 		if (!list_empty(&tid->buf_q))
 			ath_tx_queue_tid(txq, tid);
 
-		break;
+		if (tid == last || txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH)
+			break;
 	} while (!list_empty(&ac->tid_q));
 
 	if (!list_empty(&ac->tid_q)) {
-- 
1.7.3.2

[PATCH 2/2] ath9k: fix excessive BAR sending when a frame exceeds its retry limit

[Felix Fietkau]

Because the sendbar variable was not reset to zero, the stack would send
Block ACK requests for all subframes following the one that failed, which
could mess up the receiver side block ack window.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/xmit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index fffd13d..ad569e1 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -429,7 +429,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
 
 	ath_tx_count_frames(sc, bf, ts, txok, &nframes, &nbad);
 	while (bf) {
-		txfail = txpending = 0;
+		txfail = txpending = sendbar = 0;
 		bf_next = bf->bf_next;
 
 		skb = bf->bf_mpdu;
-- 
1.7.3.2

Re: [PATCH 1/2] ath9k: try more than one tid when scheduling a new aggregate

[Ben Greear]

On 01/15/2011 05:30 AM, Felix Fietkau wrote:
> Sometimes the first TID in the first AC's list is not available for forming
> a new aggregate (the BAW might not allow it), however other TIDs may have
> data available for sending.
> Prevent a slowdown of other TIDs by going through multiple entries until
> we've either hit the last one or enough AMPDUs are pending in the hardware
> queue.

I had attempted a similar patch, but I was thinking we should iterate
through all of the txq->axq_acq entries, not just use the first one?

If you agree, I can merge my patch with your patch below
and re-post.

Thanks,
Ben


 >
 > Signed-off-by: Felix Fietkau<nbd@openwrt.org>
 > ---
 >   drivers/net/wireless/ath/ath9k/xmit.c |    9 ++++++---
 >   1 files changed, 6 insertions(+), 3 deletions(-)
 >
 > diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
 > index ab4f7b4..fffd13d 100644
 > --- a/drivers/net/wireless/ath/ath9k/xmit.c
 > +++ b/drivers/net/wireless/ath/ath9k/xmit.c
 > @@ -1224,12 +1224,14 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
 >   void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
 >   {
 >   	struct ath_atx_ac *ac;
 > -	struct ath_atx_tid *tid;
 > +	struct ath_atx_tid *tid, *last;
 >
 > -	if (list_empty(&txq->axq_acq))
 > +	if (list_empty(&txq->axq_acq) ||
 > +	    txq->axq_ampdu_depth>= ATH_AGGR_MIN_QDEPTH)
 >   		return;
 >
 >   	ac = list_first_entry(&txq->axq_acq, struct ath_atx_ac, list);
 > +	last = list_entry(ac->tid_q.prev, struct ath_atx_tid, list);
 >   	list_del(&ac->list);
 >   	ac->sched = false;
 >
 > @@ -1253,7 +1255,8 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
 >   		if (!list_empty(&tid->buf_q))
 >   			ath_tx_queue_tid(txq, tid);
 >
 > -		break;
 > +		if (tid == last || txq->axq_ampdu_depth>= ATH_AGGR_MIN_QDEPTH)
 > +			break;
 >   	} while (!list_empty(&ac->tid_q));
 >
 >   	if (!list_empty(&ac->tid_q)) {

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 1/2] ath9k: try more than one tid when scheduling a new aggregate

[Felix Fietkau]

On 2011-01-19 1:26 AM, Ben Greear wrote:
> On 01/15/2011 05:30 AM, Felix Fietkau wrote:
>> Sometimes the first TID in the first AC's list is not available for forming
>> a new aggregate (the BAW might not allow it), however other TIDs may have
>> data available for sending.
>> Prevent a slowdown of other TIDs by going through multiple entries until
>> we've either hit the last one or enough AMPDUs are pending in the hardware
>> queue.
> 
> I had attempted a similar patch, but I was thinking we should iterate
> through all of the txq->axq_acq entries, not just use the first one?
> 
> If you agree, I can merge my patch with your patch below
> and re-post.
My patch was merged, how about sending an incremental patch on top of it
for review?

- Felix

Re: [PATCH 1/2] ath9k: try more than one tid when scheduling a new aggregate

[Ben Greear]

On 01/18/2011 04:33 PM, Felix Fietkau wrote:
> On 2011-01-19 1:26 AM, Ben Greear wrote:
>> On 01/15/2011 05:30 AM, Felix Fietkau wrote:
>>> Sometimes the first TID in the first AC's list is not available for forming
>>> a new aggregate (the BAW might not allow it), however other TIDs may have
>>> data available for sending.
>>> Prevent a slowdown of other TIDs by going through multiple entries until
>>> we've either hit the last one or enough AMPDUs are pending in the hardware
>>> queue.
>>
>> I had attempted a similar patch, but I was thinking we should iterate
>> through all of the txq->axq_acq entries, not just use the first one?
>>
>> If you agree, I can merge my patch with your patch below
>> and re-post.
> My patch was merged, how about sending an incremental patch on top of it
> for review?

Right...I just wanted to make sure that it was a sane thing to do
before making the effort.

I'll try to get something posted soonish.

Thanks,
Ben

>
> - Felix
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com