If station is ineligible for transmission in ieee80211_txq_may_transmit(),
no packet will be delivered to FW. During the tests in push-pull mode with
many clients, after several seconds, not a single station is an eligible
candidate for transmission since global time is smaller than all the
station's virtual airtime. As a consequence, the Tx has been blocked and
throughput is quite low.
To avoid this situation to occur in push-pull mode, the new proposal is:
- Increase the airtime grace period a little more to reduce the
unexpected sync
- If global virtual time is less than the virtual airtime of any station,
sync it to the airtime of first station in the red-black tree
- Round the division result since the process of global virtual time
involves the division calculation
Co-developed-by: Yibo Zhao <[email protected]>
Signed-off-by: Yibo Zhao <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
net/mac80211/sta_info.c | 3 ++-
net/mac80211/sta_info.h | 2 +-
net/mac80211/tx.c | 16 +++++++++++++++-
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 9d01fdd..feac975 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1852,7 +1852,8 @@ void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
weight_sum = local->airtime_weight_sum[ac] ?: sta->airtime_weight;
- local->airtime_v_t[ac] += airtime / weight_sum;
+ /* Round the calculation of global vt */
+ local->airtime_v_t[ac] += (airtime + (weight_sum >> 1)) / weight_sum;
sta->airtime[ac].v_t += airtime / sta->airtime_weight;
ieee80211_resort_txq(&local->hw, txq);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 5c1cac9..5055f94 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -130,7 +130,7 @@ enum ieee80211_agg_stop_reason {
/* Debugfs flags to enable/disable use of RX/TX airtime in scheduler */
#define AIRTIME_USE_TX BIT(0)
#define AIRTIME_USE_RX BIT(1)
-#define AIRTIME_GRACE 500 /* usec of grace period before reset */
+#define AIRTIME_GRACE 2000 /* usec of grace period before reset */
struct airtime_info {
u64 rx_airtime;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 42ca010..60cf569 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3867,15 +3867,29 @@ bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
struct ieee80211_local *local = hw_to_local(hw);
- struct txq_info *txqi = to_txq_info(txq);
+ struct txq_info *first_txqi, *txqi = to_txq_info(txq);
+ struct rb_node *node = NULL;
struct sta_info *sta;
u8 ac = txq->ac;
+ first_txqi = NULL;
lockdep_assert_held(&local->active_txq_lock[ac]);
if (!txqi->txq.sta)
return true;
+ node = rb_first_cached(&local->active_txqs[ac]);
+ if (node) {
+ first_txqi = container_of(node, struct txq_info,
+ schedule_order);
+ if (first_txqi->txq.sta) {
+ sta = container_of(first_txqi->txq.sta,
+ struct sta_info, sta);
+ if (local->airtime_v_t[ac] < sta->airtime[ac].v_t)
+ local->airtime_v_t[ac] = sta->airtime[ac].v_t;
+ }
+ }
+
sta = container_of(txqi->txq.sta, struct sta_info, sta);
return (sta->airtime[ac].v_t <= local->airtime_v_t[ac]);
}
--
1.9.1
Without really looking at the code -
> If station is ineligible for transmission in ieee80211_txq_may_transmit(),
> no packet will be delivered to FW. During the tests in push-pull mode with
> many clients, after several seconds, not a single station is an eligible
> candidate for transmission since global time is smaller than all the
> station's virtual airtime. As a consequence, the Tx has been blocked and
> throughput is quite low.
You should rewrite this to be, erm, a bit more understandable in
mac80211 context. I assume you're speaking (mostly?) about ath10k, but I
have very little context there. "push pull mode"? "firmware"? These
things are not something mac80211 knows about.
> Co-developed-by: Yibo Zhao <[email protected]>
That also seems wrong, should be Toke I guess, unless you intended for a
From: Toke to be present?
johannes
On 2019-09-16 23:27, Johannes Berg wrote:
> Without really looking at the code -
>
>> If station is ineligible for transmission in
>> ieee80211_txq_may_transmit(),
>> no packet will be delivered to FW. During the tests in push-pull mode
>> with
>> many clients, after several seconds, not a single station is an
>> eligible
>> candidate for transmission since global time is smaller than all the
>> station's virtual airtime. As a consequence, the Tx has been blocked
>> and
>> throughput is quite low.
>
> You should rewrite this to be, erm, a bit more understandable in
> mac80211 context. I assume you're speaking (mostly?) about ath10k, but
> I
> have very little context there. "push pull mode"? "firmware"? These
> things are not something mac80211 knows about.
Hi Johannes,
Thanks for your kindly reminder. Will rewrite the commit log.
>
>> Co-developed-by: Yibo Zhao <[email protected]>
>
> That also seems wrong, should be Toke I guess, unless you intended for
> a
> From: Toke to be present?
Do you mean it should be something like:
Co-developed-by: Toke Høiland-Jørgensen <[email protected]>
Signed-off-by: Yibo Zhao <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
Am I understanding right?
>
> johannes
--
Yibo
On Tue, 2019-09-17 at 14:36 +0800, Yibo Zhao wrote:
>
> Do you mean it should be something like:
>
> Co-developed-by: Toke Høiland-Jørgensen <[email protected]>
> Signed-off-by: Yibo Zhao <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
Yes, I think you mean the right thing. For the record, it seems to me it
should be
From: A <...>
[...]
Co-developed-by: B <...>
Signed-off-by: B <...>
Signed-off-by: A <...>
or so.
IOW, I think having the same "From:" (which gets preserved in git as
"Author") and "Co-developed-by" makes no sense?
Your "From" line was implied, but I suppose you did mean that From would
be yourself (as it was in the patch) and then the above seems right.
Or you can add a "From: Toke ..." to your patch message and leave the
"Co-developed-by: yourself" I suppose, the difference is in how git will
record it.
johannes
Yibo Zhao <[email protected]> writes:
> On 2019-09-16 23:27, Johannes Berg wrote:
>> Without really looking at the code -
>>
>>> If station is ineligible for transmission in
>>> ieee80211_txq_may_transmit(),
>>> no packet will be delivered to FW. During the tests in push-pull mode
>>> with
>>> many clients, after several seconds, not a single station is an
>>> eligible
>>> candidate for transmission since global time is smaller than all the
>>> station's virtual airtime. As a consequence, the Tx has been blocked
>>> and
>>> throughput is quite low.
>>
>> You should rewrite this to be, erm, a bit more understandable in
>> mac80211 context. I assume you're speaking (mostly?) about ath10k, but
>> I
>> have very little context there. "push pull mode"? "firmware"? These
>> things are not something mac80211 knows about.
> Hi Johannes,
>
> Thanks for your kindly reminder. Will rewrite the commit log.
>
>>
>>> Co-developed-by: Yibo Zhao <[email protected]>
>>
>> That also seems wrong, should be Toke I guess, unless you intended for
>> a
>> From: Toke to be present?
> Do you mean it should be something like:
>
> Co-developed-by: Toke Høiland-Jørgensen <[email protected]>
> Signed-off-by: Yibo Zhao <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>
> Am I understanding right?
I think the right thing here, as with the previous patch, is to just
drop my sign-off; you're writing this patch, and I'll add ack/reviews as
appropriate. And in that case, well, no need to have co-developed-by
yourself when your name is on the patch as author :)
-Toke
On 2019-09-18 05:12, Toke Høiland-Jørgensen wrote:
> Yibo Zhao <[email protected]> writes:
>
>> On 2019-09-16 23:27, Johannes Berg wrote:
>>> Without really looking at the code -
>>>
>>>> If station is ineligible for transmission in
>>>> ieee80211_txq_may_transmit(),
>>>> no packet will be delivered to FW. During the tests in push-pull
>>>> mode
>>>> with
>>>> many clients, after several seconds, not a single station is an
>>>> eligible
>>>> candidate for transmission since global time is smaller than all the
>>>> station's virtual airtime. As a consequence, the Tx has been blocked
>>>> and
>>>> throughput is quite low.
>>>
>>> You should rewrite this to be, erm, a bit more understandable in
>>> mac80211 context. I assume you're speaking (mostly?) about ath10k,
>>> but
>>> I
>>> have very little context there. "push pull mode"? "firmware"? These
>>> things are not something mac80211 knows about.
>> Hi Johannes,
>>
>> Thanks for your kindly reminder. Will rewrite the commit log.
>>
>>>
>>>> Co-developed-by: Yibo Zhao <[email protected]>
>>>
>>> That also seems wrong, should be Toke I guess, unless you intended
>>> for
>>> a
>>> From: Toke to be present?
>> Do you mean it should be something like:
>>
>> Co-developed-by: Toke Høiland-Jørgensen <[email protected]>
>> Signed-off-by: Yibo Zhao <[email protected]>
>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>>
>> Am I understanding right?
>
> I think the right thing here, as with the previous patch, is to just
> drop my sign-off; you're writing this patch, and I'll add ack/reviews
> as
> appropriate. And in that case, well, no need to have co-developed-by
> yourself when your name is on the patch as author :)
>
> -Toke
Sorry, I think I have missed checking your reply, please ignore the
wrong signed-off in PATCH-V2.
--
Yibo
Yibo Zhao <[email protected]> writes:
> On 2019-09-18 05:12, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao <[email protected]> writes:
>>
>>> On 2019-09-16 23:27, Johannes Berg wrote:
>>>> Without really looking at the code -
>>>>
>>>>> If station is ineligible for transmission in
>>>>> ieee80211_txq_may_transmit(),
>>>>> no packet will be delivered to FW. During the tests in push-pull
>>>>> mode
>>>>> with
>>>>> many clients, after several seconds, not a single station is an
>>>>> eligible
>>>>> candidate for transmission since global time is smaller than all the
>>>>> station's virtual airtime. As a consequence, the Tx has been blocked
>>>>> and
>>>>> throughput is quite low.
>>>>
>>>> You should rewrite this to be, erm, a bit more understandable in
>>>> mac80211 context. I assume you're speaking (mostly?) about ath10k,
>>>> but
>>>> I
>>>> have very little context there. "push pull mode"? "firmware"? These
>>>> things are not something mac80211 knows about.
>>> Hi Johannes,
>>>
>>> Thanks for your kindly reminder. Will rewrite the commit log.
>>>
>>>>
>>>>> Co-developed-by: Yibo Zhao <[email protected]>
>>>>
>>>> That also seems wrong, should be Toke I guess, unless you intended
>>>> for
>>>> a
>>>> From: Toke to be present?
>>> Do you mean it should be something like:
>>>
>>> Co-developed-by: Toke Høiland-Jørgensen <[email protected]>
>>> Signed-off-by: Yibo Zhao <[email protected]>
>>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>>>
>>> Am I understanding right?
>>
>> I think the right thing here, as with the previous patch, is to just
>> drop my sign-off; you're writing this patch, and I'll add ack/reviews
>> as
>> appropriate. And in that case, well, no need to have co-developed-by
>> yourself when your name is on the patch as author :)
>>
>> -Toke
> Sorry, I think I have missed checking your reply, please ignore the
> wrong signed-off in PATCH-V2.
While you're re-spinning, could you please add a changelog for the
changes you make? Makes it easier to keep track :)
You can add a cover-letter with a full changelog instead of having a
separate changelog for each patch; that's what I usually do...
-Toke
On 2019-09-18 18:16, Toke Høiland-Jørgensen wrote:
> Yibo Zhao <[email protected]> writes:
>
>> On 2019-09-18 05:12, Toke Høiland-Jørgensen wrote:
>>> Yibo Zhao <[email protected]> writes:
>>>
>>>> On 2019-09-16 23:27, Johannes Berg wrote:
>>>>> Without really looking at the code -
>>>>>
>>>>>> If station is ineligible for transmission in
>>>>>> ieee80211_txq_may_transmit(),
>>>>>> no packet will be delivered to FW. During the tests in push-pull
>>>>>> mode
>>>>>> with
>>>>>> many clients, after several seconds, not a single station is an
>>>>>> eligible
>>>>>> candidate for transmission since global time is smaller than all
>>>>>> the
>>>>>> station's virtual airtime. As a consequence, the Tx has been
>>>>>> blocked
>>>>>> and
>>>>>> throughput is quite low.
>>>>>
>>>>> You should rewrite this to be, erm, a bit more understandable in
>>>>> mac80211 context. I assume you're speaking (mostly?) about ath10k,
>>>>> but
>>>>> I
>>>>> have very little context there. "push pull mode"? "firmware"? These
>>>>> things are not something mac80211 knows about.
>>>> Hi Johannes,
>>>>
>>>> Thanks for your kindly reminder. Will rewrite the commit log.
>>>>
>>>>>
>>>>>> Co-developed-by: Yibo Zhao <[email protected]>
>>>>>
>>>>> That also seems wrong, should be Toke I guess, unless you intended
>>>>> for
>>>>> a
>>>>> From: Toke to be present?
>>>> Do you mean it should be something like:
>>>>
>>>> Co-developed-by: Toke Høiland-Jørgensen <[email protected]>
>>>> Signed-off-by: Yibo Zhao <[email protected]>
>>>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>>>>
>>>> Am I understanding right?
>>>
>>> I think the right thing here, as with the previous patch, is to just
>>> drop my sign-off; you're writing this patch, and I'll add ack/reviews
>>> as
>>> appropriate. And in that case, well, no need to have co-developed-by
>>> yourself when your name is on the patch as author :)
>>>
>>> -Toke
>> Sorry, I think I have missed checking your reply, please ignore the
>> wrong signed-off in PATCH-V2.
>
> While you're re-spinning, could you please add a changelog for the
> changes you make? Makes it easier to keep track :)
>
> You can add a cover-letter with a full changelog instead of having a
> separate changelog for each patch; that's what I usually do...
>
> -Toke
Sure, thanks.
--
Yibo