2011-01-15 13:30:22

by Felix Fietkau

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

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 <[email protected]>
---
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



2011-01-15 13:30:22

by Felix Fietkau

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

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 <[email protected]>
---
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


2011-01-19 00:27:01

by Ben Greear

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

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<[email protected]>
> ---
> 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 <[email protected]>
Candela Technologies Inc http://www.candelatech.com


2011-01-19 00:33:46

by Felix Fietkau

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

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

2011-01-19 00:43:23

by Ben Greear

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

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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com