[PATCH] ath9k: optimize tx status update

[PATCH] ath9k: optimize tx status update

[Rajkumar Manoharan]

The recent changes of ath_tx_rc_status makes it to be called
only for the first frame of aggregation or if there is a
single subframe and for all normal frames. And also for all
aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
patch removes the unnecessary checks and make the ampdu_[ack]_len
to be filled on both aggregation and normal frames.

Signed-off-by: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/rc.c   |   11 -----------
 drivers/net/wireless/ath/ath9k/xmit.c |    5 ++---
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 4f13018..81c48fb 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1354,20 +1354,9 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
 	if (!priv_sta || !ieee80211_is_data(fc))
 		return;
 
-	/* This packet was aggregated but doesn't carry status info */
-	if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) &&
-	    !(tx_info->flags & IEEE80211_TX_STAT_AMPDU))
-		return;
-
 	if (tx_info->flags & IEEE80211_TX_STAT_TX_FILTERED)
 		return;
 
-	if (!(tx_info->flags & IEEE80211_TX_STAT_AMPDU)) {
-		tx_info->status.ampdu_ack_len =
-			(tx_info->flags & IEEE80211_TX_STAT_ACK ? 1 : 0);
-		tx_info->status.ampdu_len = 1;
-	}
-
 	if (!(tx_info->flags & IEEE80211_TX_STAT_ACK))
 		tx_status = 1;
 
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index fa3dcfd..f5d4764 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2043,10 +2043,9 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf,
 		tx_info->flags |= IEEE80211_TX_STAT_AMPDU;
 
 		BUG_ON(nbad > nframes);
-
-		tx_info->status.ampdu_len = nframes;
-		tx_info->status.ampdu_ack_len = nframes - nbad;
 	}
+	tx_info->status.ampdu_len = nframes;
+	tx_info->status.ampdu_ack_len = nframes - nbad;
 
 	if ((ts->ts_status & ATH9K_TXERR_FILT) == 0 &&
 	    (tx_info->flags & IEEE80211_TX_CTL_NO_ACK) == 0) {
-- 
1.7.6.4

Re: [PATCH] ath9k: optimize tx status update

[Felix Fietkau]

On 2011-09-26 3:42 AM, Rajkumar Manoharan wrote:
> The recent changes of ath_tx_rc_status makes it to be called
> only for the first frame of aggregation or if there is a
> single subframe and for all normal frames. And also for all
> aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
> patch removes the unnecessary checks and make the ampdu_[ack]_len
> to be filled on both aggregation and normal frames.
This patch looks wrong to me. Only one subframe per A-MPDU is supposed 
to carry the IEEE80211_TX_STAT_AMPDU flag, so the check you removed from 
rc.c is actually necessary, otherwise it'll process the tx status of an 
A-MPDU multiple times (once per completed subframe).

Re: [PATCH] ath9k: optimize tx status update

[Rajkumar Manoharan]

On Mon, Sep 26, 2011 at 07:27:43AM -0600, Felix Fietkau wrote:
> On 2011-09-26 3:42 AM, Rajkumar Manoharan wrote:
> >The recent changes of ath_tx_rc_status makes it to be called
> >only for the first frame of aggregation or if there is a
> >single subframe and for all normal frames. And also for all
> >aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
> >patch removes the unnecessary checks and make the ampdu_[ack]_len
> >to be filled on both aggregation and normal frames.
> This patch looks wrong to me. Only one subframe per A-MPDU is
> supposed to carry the IEEE80211_TX_STAT_AMPDU flag, so the check you
> removed from rc.c is actually necessary, otherwise it'll process the
> tx status of an A-MPDU multiple times (once per completed subframe).
> 
I dont understand. Right now ath_tx_rc_status is called only for the first
frame of aggregation or there is only one subframe in the aggregation.
It never be called for all subframes. Am i right?

--
Rajkumar

Re: [PATCH] ath9k: optimize tx status update

[Felix Fietkau]

On 26.09.2011, at 17:12, Rajkumar Manoharan <rmanohar@qca.qualcomm.com> wrote:

> On Mon, Sep 26, 2011 at 07:27:43AM -0600, Felix Fietkau wrote:
>> On 2011-09-26 3:42 AM, Rajkumar Manoharan wrote:
>>> The recent changes of ath_tx_rc_status makes it to be called
>>> only for the first frame of aggregation or if there is a
>>> single subframe and for all normal frames. And also for all
>>> aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
>>> patch removes the unnecessary checks and make the ampdu_[ack]_len
>>> to be filled on both aggregation and normal frames.
>> This patch looks wrong to me. Only one subframe per A-MPDU is
>> supposed to carry the IEEE80211_TX_STAT_AMPDU flag, so the check you
>> removed from rc.c is actually necessary, otherwise it'll process the
>> tx status of an A-MPDU multiple times (once per completed subframe).
>> 
> I dont understand. Right now ath_tx_rc_status is called only for the first
> frame of aggregation or there is only one subframe in the aggregation.
> It never be called for all subframes. Am i right?
Right, but the rate control status update is called per subframe, so the rc.c change is broken