Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
On 2019-09-21 22:00, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-21 21:02, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-21 19:27, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
In a loop txqs dequeue scenario, if the first txq in the
rbtree
gets
removed from rbtree immediately in the
ieee80211_return_txq(),
the
loop will break soon in the ieee80211_next_txq() due to
schedule_pos
not leading to the second txq in the rbtree. Thus,
defering
the
removal right before the end of this schedule round.
Co-developed-by: Yibo Zhao
Signed-off-by: Yibo Zhao
Signed-off-by: Toke Høiland-Jørgensen
I didn't write this patch, so please don't use my sign-off.
I'll
add
ack or review tags as appropriate in reply; but a few
comments
first:
---
include/net/mac80211.h | 16 ++--
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/main.c| 6 +
net/mac80211/tx.c | 63
+++---
4 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/include/net/mac80211.h
b/include/net/mac80211.h
index ac2ed8e..ba5a345 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
#define IEEE80211_MAX_TX_RETRY 31
+#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
+
static inline void ieee80211_rate_set_vht(struct
ieee80211_tx_rate
*rate,
u8 mcs, u8 nss)
{
@@ -6232,7 +6234,8 @@ struct sk_buff
*ieee80211_tx_dequeue(struct
ieee80211_hw *hw,
* @ac: AC number to return packets from.
*
* Should only be called between calls to
ieee80211_txq_schedule_start()
- * and ieee80211_txq_schedule_end().
+ * and ieee80211_txq_schedule_end(). If the txq is empty,
it
will
be
added
+ * to a remove list and get removed later.
* Returns the next txq if successful, %NULL if no queue
is
eligible.
If a txq
* is returned, it should be returned with
ieee80211_return_txq()
after the
* driver has finished scheduling it.
@@ -6268,7 +6271,8 @@ void
ieee80211_txq_schedule_start(struct
ieee80211_hw *hw, u8 ac)
* @hw: pointer as obtained from ieee80211_alloc_hw()
* @ac: AC number to acquire locks for
*
- * Release locks previously acquired by
ieee80211_txq_schedule_end().
+ * Release locks previously acquired by
ieee80211_txq_schedule_end().
Check
+ * and remove the empty txq from rb-tree.
*/
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw,
u8
ac)
__releases(txq_lock);
@@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
ieee80211_hw
*hw, struct ieee80211_txq *txq)
__acquires(txq_lock) __releases(txq_lock);
/**
+ * ieee80211_txqs_check - Check txqs waiting for removal
+ *
+ * @tmr: pointer as obtained from local
+ *
+ */
+void ieee80211_txqs_check(struct timer_list *tmr);
+
+/**
* ieee80211_txq_may_transmit - check whether TXQ is
allowed
to
transmit
*
* This function is used to check whether given txq is
allowed
to
transmit by
diff --git a/net/mac80211/ieee80211_i.h
b/net/mac80211/ieee80211_i.h
index a4556f9..49aa143e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -847,6 +847,7 @@ struct txq_info {
struct codel_stats cstats;
struct sk_buff_head frags;
struct rb_node schedule_order;
+ struct list_head candidate;
unsigned long flags;
/* keep last! */
@@ -1145,6 +1146,8 @@ struct ieee80211_local {
u64 airtime_v_t[IEEE80211_NUM_ACS];
u64 airtime_weight_sum[IEEE80211_NUM_ACS];
+ struct list_head remove_list[IEEE80211_NUM_ACS];
+ struct timer_list remove_timer;
u16 airtime_flags;
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index e9ffa8e..78fe24a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -667,10 +667,15 @@ struct ieee80211_hw
*ieee80211_alloc_hw_nm(size_t priv_data_len,
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
local->active_txqs[i] = RB_ROOT_CACHED;
+ INIT_LIST_HEAD(>remove_list[i]);
spin_lock_init(>active_txq_lock[i]);
}
local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
+ timer_setup(>remove_timer, ieee80211_txqs_check,
0);
+ mod_timer(>remove_timer,
+ jiffies +
msecs_to_jiffies(IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS));
+
INIT_LIST_HEAD(>chanctx_list);
mutex_init(>chanctx_mtx);
@@ -1305,6 +1310,7 @@ void ieee80211_unregister_hw(struct
ieee80211_hw
*hw)
tasklet_kill(>tx_pending_tasklet);
tasklet_kill(>tasklet);
+ del_timer_sync(>remove_timer);
#ifdef CONFIG_INET
Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
Yibo Zhao writes:
> On 2019-09-21 21:02, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> On 2019-09-21 19:27, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
> On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
> On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
> In a loop txqs dequeue scenario, if the first txq in the
> rbtree
> gets
> removed from rbtree immediately in the
> ieee80211_return_txq(),
> the
> loop will break soon in the ieee80211_next_txq() due to
> schedule_pos
> not leading to the second txq in the rbtree. Thus, defering
> the
> removal right before the end of this schedule round.
>
> Co-developed-by: Yibo Zhao
> Signed-off-by: Yibo Zhao
> Signed-off-by: Toke Høiland-Jørgensen
I didn't write this patch, so please don't use my sign-off.
I'll
add
ack or review tags as appropriate in reply; but a few
comments
first:
> ---
> include/net/mac80211.h | 16 ++--
> net/mac80211/ieee80211_i.h | 3 +++
> net/mac80211/main.c| 6 +
> net/mac80211/tx.c | 63
> +++---
> 4 files changed, 83 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index ac2ed8e..ba5a345 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
>
> #define IEEE80211_MAX_TX_RETRY 31
>
> +#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
> +
> static inline void ieee80211_rate_set_vht(struct
> ieee80211_tx_rate
> *rate,
> u8 mcs, u8 nss)
> {
> @@ -6232,7 +6234,8 @@ struct sk_buff
> *ieee80211_tx_dequeue(struct
> ieee80211_hw *hw,
> * @ac: AC number to return packets from.
> *
> * Should only be called between calls to
> ieee80211_txq_schedule_start()
> - * and ieee80211_txq_schedule_end().
> + * and ieee80211_txq_schedule_end(). If the txq is empty,
> it
> will
> be
> added
> + * to a remove list and get removed later.
> * Returns the next txq if successful, %NULL if no queue is
> eligible.
> If a txq
> * is returned, it should be returned with
> ieee80211_return_txq()
> after the
> * driver has finished scheduling it.
> @@ -6268,7 +6271,8 @@ void
> ieee80211_txq_schedule_start(struct
> ieee80211_hw *hw, u8 ac)
> * @hw: pointer as obtained from ieee80211_alloc_hw()
> * @ac: AC number to acquire locks for
> *
> - * Release locks previously acquired by
> ieee80211_txq_schedule_end().
> + * Release locks previously acquired by
> ieee80211_txq_schedule_end().
> Check
> + * and remove the empty txq from rb-tree.
> */
> void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8
> ac)
> __releases(txq_lock);
> @@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
> ieee80211_hw
> *hw, struct ieee80211_txq *txq)
> __acquires(txq_lock) __releases(txq_lock);
>
> /**
> + * ieee80211_txqs_check - Check txqs waiting for removal
> + *
> + * @tmr: pointer as obtained from local
> + *
> + */
> +void ieee80211_txqs_check(struct timer_list *tmr);
> +
> +/**
> * ieee80211_txq_may_transmit - check whether TXQ is
> allowed
> to
> transmit
> *
> * This function is used to check whether given txq is
> allowed
> to
> transmit by
> diff --git a/net/mac80211/ieee80211_i.h
> b/net/mac80211/ieee80211_i.h
> index a4556f9..49aa143e 100644
> ---
Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
On 2019-09-21 21:02, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-21 19:27, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
In a loop txqs dequeue scenario, if the first txq in the
rbtree
gets
removed from rbtree immediately in the
ieee80211_return_txq(),
the
loop will break soon in the ieee80211_next_txq() due to
schedule_pos
not leading to the second txq in the rbtree. Thus, defering
the
removal right before the end of this schedule round.
Co-developed-by: Yibo Zhao
Signed-off-by: Yibo Zhao
Signed-off-by: Toke Høiland-Jørgensen
I didn't write this patch, so please don't use my sign-off.
I'll
add
ack or review tags as appropriate in reply; but a few
comments
first:
---
include/net/mac80211.h | 16 ++--
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/main.c| 6 +
net/mac80211/tx.c | 63
+++---
4 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ac2ed8e..ba5a345 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
#define IEEE80211_MAX_TX_RETRY 31
+#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
+
static inline void ieee80211_rate_set_vht(struct
ieee80211_tx_rate
*rate,
u8 mcs, u8 nss)
{
@@ -6232,7 +6234,8 @@ struct sk_buff
*ieee80211_tx_dequeue(struct
ieee80211_hw *hw,
* @ac: AC number to return packets from.
*
* Should only be called between calls to
ieee80211_txq_schedule_start()
- * and ieee80211_txq_schedule_end().
+ * and ieee80211_txq_schedule_end(). If the txq is empty,
it
will
be
added
+ * to a remove list and get removed later.
* Returns the next txq if successful, %NULL if no queue is
eligible.
If a txq
* is returned, it should be returned with
ieee80211_return_txq()
after the
* driver has finished scheduling it.
@@ -6268,7 +6271,8 @@ void
ieee80211_txq_schedule_start(struct
ieee80211_hw *hw, u8 ac)
* @hw: pointer as obtained from ieee80211_alloc_hw()
* @ac: AC number to acquire locks for
*
- * Release locks previously acquired by
ieee80211_txq_schedule_end().
+ * Release locks previously acquired by
ieee80211_txq_schedule_end().
Check
+ * and remove the empty txq from rb-tree.
*/
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8
ac)
__releases(txq_lock);
@@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
ieee80211_hw
*hw, struct ieee80211_txq *txq)
__acquires(txq_lock) __releases(txq_lock);
/**
+ * ieee80211_txqs_check - Check txqs waiting for removal
+ *
+ * @tmr: pointer as obtained from local
+ *
+ */
+void ieee80211_txqs_check(struct timer_list *tmr);
+
+/**
* ieee80211_txq_may_transmit - check whether TXQ is
allowed
to
transmit
*
* This function is used to check whether given txq is
allowed
to
transmit by
diff --git a/net/mac80211/ieee80211_i.h
b/net/mac80211/ieee80211_i.h
index a4556f9..49aa143e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -847,6 +847,7 @@ struct txq_info {
struct codel_stats cstats;
struct sk_buff_head frags;
struct rb_node schedule_order;
+ struct list_head candidate;
unsigned long flags;
/* keep last! */
@@ -1145,6 +1146,8 @@ struct ieee80211_local {
u64 airtime_v_t[IEEE80211_NUM_ACS];
u64 airtime_weight_sum[IEEE80211_NUM_ACS];
+ struct list_head remove_list[IEEE80211_NUM_ACS];
+ struct timer_list remove_timer;
u16 airtime_flags;
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index e9ffa8e..78fe24a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -667,10 +667,15 @@ struct ieee80211_hw
*ieee80211_alloc_hw_nm(size_t priv_data_len,
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
local->active_txqs[i] = RB_ROOT_CACHED;
+ INIT_LIST_HEAD(>remove_list[i]);
spin_lock_init(>active_txq_lock[i]);
}
local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
+ timer_setup(>remove_timer, ieee80211_txqs_check,
0);
+ mod_timer(>remove_timer,
+ jiffies +
msecs_to_jiffies(IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS));
+
INIT_LIST_HEAD(>chanctx_list);
mutex_init(>chanctx_mtx);
@@ -1305,6 +1310,7 @@ void ieee80211_unregister_hw(struct
ieee80211_hw
*hw)
tasklet_kill(>tx_pending_tasklet);
tasklet_kill(>tasklet);
+ del_timer_sync(>remove_timer);
#ifdef CONFIG_INET
unregister_inetaddr_notifier(>ifa_notifier);
#endif
diff --git
Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
Yibo Zhao writes:
> On 2019-09-21 19:27, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
> On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
> On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> In a loop txqs dequeue scenario, if the first txq in the
>>> rbtree
>>> gets
>>> removed from rbtree immediately in the ieee80211_return_txq(),
>>> the
>>> loop will break soon in the ieee80211_next_txq() due to
>>> schedule_pos
>>> not leading to the second txq in the rbtree. Thus, defering
>>> the
>>> removal right before the end of this schedule round.
>>>
>>> Co-developed-by: Yibo Zhao
>>> Signed-off-by: Yibo Zhao
>>> Signed-off-by: Toke Høiland-Jørgensen
>>
>> I didn't write this patch, so please don't use my sign-off.
>> I'll
>> add
>> ack or review tags as appropriate in reply; but a few comments
>> first:
>>
>>> ---
>>> include/net/mac80211.h | 16 ++--
>>> net/mac80211/ieee80211_i.h | 3 +++
>>> net/mac80211/main.c| 6 +
>>> net/mac80211/tx.c | 63
>>> +++---
>>> 4 files changed, 83 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>>> index ac2ed8e..ba5a345 100644
>>> --- a/include/net/mac80211.h
>>> +++ b/include/net/mac80211.h
>>> @@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
>>>
>>> #define IEEE80211_MAX_TX_RETRY 31
>>>
>>> +#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
>>> +
>>> static inline void ieee80211_rate_set_vht(struct
>>> ieee80211_tx_rate
>>> *rate,
>>> u8 mcs, u8 nss)
>>> {
>>> @@ -6232,7 +6234,8 @@ struct sk_buff
>>> *ieee80211_tx_dequeue(struct
>>> ieee80211_hw *hw,
>>> * @ac: AC number to return packets from.
>>> *
>>> * Should only be called between calls to
>>> ieee80211_txq_schedule_start()
>>> - * and ieee80211_txq_schedule_end().
>>> + * and ieee80211_txq_schedule_end(). If the txq is empty, it
>>> will
>>> be
>>> added
>>> + * to a remove list and get removed later.
>>> * Returns the next txq if successful, %NULL if no queue is
>>> eligible.
>>> If a txq
>>> * is returned, it should be returned with
>>> ieee80211_return_txq()
>>> after the
>>> * driver has finished scheduling it.
>>> @@ -6268,7 +6271,8 @@ void ieee80211_txq_schedule_start(struct
>>> ieee80211_hw *hw, u8 ac)
>>> * @hw: pointer as obtained from ieee80211_alloc_hw()
>>> * @ac: AC number to acquire locks for
>>> *
>>> - * Release locks previously acquired by
>>> ieee80211_txq_schedule_end().
>>> + * Release locks previously acquired by
>>> ieee80211_txq_schedule_end().
>>> Check
>>> + * and remove the empty txq from rb-tree.
>>> */
>>> void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8
>>> ac)
>>> __releases(txq_lock);
>>> @@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
>>> ieee80211_hw
>>> *hw, struct ieee80211_txq *txq)
>>> __acquires(txq_lock) __releases(txq_lock);
>>>
>>> /**
>>> + * ieee80211_txqs_check - Check txqs waiting for removal
>>> + *
>>> + * @tmr: pointer as obtained from local
>>> + *
>>> + */
>>> +void ieee80211_txqs_check(struct timer_list *tmr);
>>> +
>>> +/**
>>> * ieee80211_txq_may_transmit - check whether TXQ is allowed
>>> to
>>> transmit
>>> *
>>> * This function is used to check whether given txq is
>>> allowed
>>> to
>>> transmit by
>>> diff --git a/net/mac80211/ieee80211_i.h
>>> b/net/mac80211/ieee80211_i.h
>>> index a4556f9..49aa143e 100644
>>> --- a/net/mac80211/ieee80211_i.h
>>> +++ b/net/mac80211/ieee80211_i.h
>>> @@ -847,6 +847,7 @@ struct txq_info {
>>> struct codel_stats cstats;
>>> struct sk_buff_head frags;
>>> struct rb_node schedule_order;
>>> + struct list_head candidate;
>>> unsigned long flags;
>>>
>>> /* keep last! */
>>>
Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
On 2019-09-21 19:27, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
In a loop txqs dequeue scenario, if the first txq in the
rbtree
gets
removed from rbtree immediately in the ieee80211_return_txq(),
the
loop will break soon in the ieee80211_next_txq() due to
schedule_pos
not leading to the second txq in the rbtree. Thus, defering
the
removal right before the end of this schedule round.
Co-developed-by: Yibo Zhao
Signed-off-by: Yibo Zhao
Signed-off-by: Toke Høiland-Jørgensen
I didn't write this patch, so please don't use my sign-off.
I'll
add
ack or review tags as appropriate in reply; but a few comments
first:
---
include/net/mac80211.h | 16 ++--
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/main.c| 6 +
net/mac80211/tx.c | 63
+++---
4 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ac2ed8e..ba5a345 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
#define IEEE80211_MAX_TX_RETRY 31
+#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
+
static inline void ieee80211_rate_set_vht(struct
ieee80211_tx_rate
*rate,
u8 mcs, u8 nss)
{
@@ -6232,7 +6234,8 @@ struct sk_buff
*ieee80211_tx_dequeue(struct
ieee80211_hw *hw,
* @ac: AC number to return packets from.
*
* Should only be called between calls to
ieee80211_txq_schedule_start()
- * and ieee80211_txq_schedule_end().
+ * and ieee80211_txq_schedule_end(). If the txq is empty, it
will
be
added
+ * to a remove list and get removed later.
* Returns the next txq if successful, %NULL if no queue is
eligible.
If a txq
* is returned, it should be returned with
ieee80211_return_txq()
after the
* driver has finished scheduling it.
@@ -6268,7 +6271,8 @@ void ieee80211_txq_schedule_start(struct
ieee80211_hw *hw, u8 ac)
* @hw: pointer as obtained from ieee80211_alloc_hw()
* @ac: AC number to acquire locks for
*
- * Release locks previously acquired by
ieee80211_txq_schedule_end().
+ * Release locks previously acquired by
ieee80211_txq_schedule_end().
Check
+ * and remove the empty txq from rb-tree.
*/
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8
ac)
__releases(txq_lock);
@@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
ieee80211_hw
*hw, struct ieee80211_txq *txq)
__acquires(txq_lock) __releases(txq_lock);
/**
+ * ieee80211_txqs_check - Check txqs waiting for removal
+ *
+ * @tmr: pointer as obtained from local
+ *
+ */
+void ieee80211_txqs_check(struct timer_list *tmr);
+
+/**
* ieee80211_txq_may_transmit - check whether TXQ is allowed
to
transmit
*
* This function is used to check whether given txq is
allowed
to
transmit by
diff --git a/net/mac80211/ieee80211_i.h
b/net/mac80211/ieee80211_i.h
index a4556f9..49aa143e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -847,6 +847,7 @@ struct txq_info {
struct codel_stats cstats;
struct sk_buff_head frags;
struct rb_node schedule_order;
+ struct list_head candidate;
unsigned long flags;
/* keep last! */
@@ -1145,6 +1146,8 @@ struct ieee80211_local {
u64 airtime_v_t[IEEE80211_NUM_ACS];
u64 airtime_weight_sum[IEEE80211_NUM_ACS];
+ struct list_head remove_list[IEEE80211_NUM_ACS];
+ struct timer_list remove_timer;
u16 airtime_flags;
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index e9ffa8e..78fe24a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -667,10 +667,15 @@ struct ieee80211_hw
*ieee80211_alloc_hw_nm(size_t priv_data_len,
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
local->active_txqs[i] = RB_ROOT_CACHED;
+ INIT_LIST_HEAD(>remove_list[i]);
spin_lock_init(>active_txq_lock[i]);
}
local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
+ timer_setup(>remove_timer, ieee80211_txqs_check, 0);
+ mod_timer(>remove_timer,
+ jiffies +
msecs_to_jiffies(IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS));
+
INIT_LIST_HEAD(>chanctx_list);
mutex_init(>chanctx_mtx);
@@ -1305,6 +1310,7 @@ void ieee80211_unregister_hw(struct
ieee80211_hw
*hw)
tasklet_kill(>tx_pending_tasklet);
tasklet_kill(>tasklet);
+ del_timer_sync(>remove_timer);
#ifdef CONFIG_INET
unregister_inetaddr_notifier(>ifa_notifier);
#endif
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index d00baaa..42ca010 100644
---
Re: [PATCH v5 2/8] ath10k: enable RX bundle receive for sdio
Wen Gong writes:
> From: Alagu Sankar
>
> The existing implementation of initiating multiple sdio transfers for
> receive bundling is slowing down the receive speed. Combining the
> transfers using a bundle method would be ideal.
>
> The transmission utilization ratio for sdio bus for small packet is
> slow, because the space and time cost for sdio bus is same for large
> length packet and small length packet. So the speed of data for large
> length packet is higher than small length.
>
> Test result of different length of data:
> data packet(byte) cost time(us) calculated rate(Mbps)
> 256 2873
> 512 33 124
> 1024 35 234
> 1792 45 318
> 14336 168 682
> 28672 333 688
> 57344 660 695
>
> Tested with QCA6174 SDIO with firmware
> WLAN.RMH.4.4.1-7-QCARMSWP-1.
>
> Signed-off-by: Alagu Sankar
> Signed-off-by: Wen Gong
[...]
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -24,6 +24,9 @@
> #include "trace.h"
> #include "sdio.h"
>
> +#define ATH10K_SDIO_DMA_BUF_SIZE (32 * 1024)
> +#define ATH10K_SDIO_VSG_BUF_SIZE (32 * 1024)
Why two defines? Seems error prone to me and using the latter should be
enough.
> @@ -529,6 +532,7 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
> size_t full_len, act_len;
> bool last_in_bundle;
> int ret, i;
> + int pkt_cnt = 0;
>
> if (n_lookaheads > ATH10K_SDIO_MAX_RX_MSGS) {
> ath10k_warn(ar,
> @@ -572,20 +576,22 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
>*/
> size_t bndl_cnt;
>
> - ret = ath10k_sdio_mbox_alloc_pkt_bundle(ar,
> -
> _sdio->rx_pkts[i],
> - htc_hdr,
> - full_len,
> - act_len,
> - _cnt);
> + struct ath10k_sdio_rx_data *rx_pkts =
> + _sdio->rx_pkts[pkt_cnt];
You need to declare rx_pkts in the beginning of the block, not mixed
within the code.
> @@ -606,9 +612,10 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
> ath10k_warn(ar, "alloc_rx_pkt error %d\n", ret);
> goto err;
> }
> + pkt_cnt++;
Empty line before 'pkt_cnt++'.
> -static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
> +static int ath10k_sdio_mbox_rx_fetch_bundle(struct ath10k *ar)
> {
> struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> + struct ath10k_sdio_rx_data *pkt;
> + struct ath10k_htc_hdr *htc_hdr;
> int ret, i;
> + u32 pkt_offset, virt_pkt_len;
>
> + virt_pkt_len = 0;
> for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
> - ret = ath10k_sdio_mbox_rx_packet(ar,
> - _sdio->rx_pkts[i]);
> + virt_pkt_len += ar_sdio->rx_pkts[i].alloc_len;
> + }
> +
> + if (virt_pkt_len < ATH10K_SDIO_DMA_BUF_SIZE) {
> + ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
> + ar_sdio->vsg_buffer, virt_pkt_len);
> if (ret)
> goto err;
> + } else {
> + ath10k_err(ar, "size exceeding limit %d\n", virt_pkt_len);
> + ret = -ENOMEM;
> + goto err;
> + }
Use common error handling style, ath10k_warn() and -E2BIG:
if (virt_pkt_len >= ATH10K_SDIO_DMA_BUF_SIZE) {
ath10k_err(ar, "size exceeding limit %d\n", virt_pkt_len);
ret = -E2BIG;
goto err;
}
ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
ar_sdio->vsg_buffer, virt_pkt_len);
if (ret) {
ath10k_warn("failed to do foo: %d", ret)
goto err;
}
> @@ -1123,7 +1151,7 @@ static int ath10k_sdio_bmi_get_rx_lookahead(struct
> ath10k *ar)
>MBOX_HOST_INT_STATUS_ADDRESS,
>_word);
> if (ret) {
> - ath10k_warn(ar, "unable to read RX_LOOKAHEAD_VALID:
> %d\n", ret);
> + ath10k_warn(ar, "unable to read rx_lookahd: %d\n", ret);
Looks like an unnecessary change?
> @@ -196,6 +196,9 @@ struct ath10k_sdio {
> struct ath10k *ar;
> struct ath10k_sdio_irq_data irq_data;
>
> + /* temporary buffer for sdio read */
> + u8 *vsg_buffer;
So how is vsg_buffer protected? You should document that here.
--
Kalle Valo
Re: [PATCH v5 5/8] ath10k: disable TX complete indication of htt for sdio
Wen Gong writes: > Tx complete message from firmware cost bus bandwidth of sdio, and bus > bandwidth is the bollteneck of throughput, it will effect the bandwidth > occupancy of data packet of TX and RX. TBH I'm not enthuastic about this, this feels an ugly hack. And it adds yet another module parameter which I detest. > This patch disable TX complete indication from firmware for htt data > packet, it results in significant performance improvement on TX path. So how much does this feature improve through exactly? Do you have any numbers? > The downside of this patch is ath10k will not know the TX status of > the data packet for poor signal situation. Although upper network stack > or application layer have retry mechanism, the retry will be later than > ath10k get the TX fail status if not disable TX complete. I don't understand this description. What's the difference in practise from user's point of view? But I think this patch should dropped from the patchset and revisited after rest of the patches are applied. -- Kalle Valo
Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
On 2019-09-21 19:27, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
In a loop txqs dequeue scenario, if the first txq in the
rbtree
gets
removed from rbtree immediately in the ieee80211_return_txq(),
the
loop will break soon in the ieee80211_next_txq() due to
schedule_pos
not leading to the second txq in the rbtree. Thus, defering
the
removal right before the end of this schedule round.
Co-developed-by: Yibo Zhao
Signed-off-by: Yibo Zhao
Signed-off-by: Toke Høiland-Jørgensen
I didn't write this patch, so please don't use my sign-off.
I'll
add
ack or review tags as appropriate in reply; but a few comments
first:
---
include/net/mac80211.h | 16 ++--
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/main.c| 6 +
net/mac80211/tx.c | 63
+++---
4 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ac2ed8e..ba5a345 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
#define IEEE80211_MAX_TX_RETRY 31
+#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
+
static inline void ieee80211_rate_set_vht(struct
ieee80211_tx_rate
*rate,
u8 mcs, u8 nss)
{
@@ -6232,7 +6234,8 @@ struct sk_buff
*ieee80211_tx_dequeue(struct
ieee80211_hw *hw,
* @ac: AC number to return packets from.
*
* Should only be called between calls to
ieee80211_txq_schedule_start()
- * and ieee80211_txq_schedule_end().
+ * and ieee80211_txq_schedule_end(). If the txq is empty, it
will
be
added
+ * to a remove list and get removed later.
* Returns the next txq if successful, %NULL if no queue is
eligible.
If a txq
* is returned, it should be returned with
ieee80211_return_txq()
after the
* driver has finished scheduling it.
@@ -6268,7 +6271,8 @@ void ieee80211_txq_schedule_start(struct
ieee80211_hw *hw, u8 ac)
* @hw: pointer as obtained from ieee80211_alloc_hw()
* @ac: AC number to acquire locks for
*
- * Release locks previously acquired by
ieee80211_txq_schedule_end().
+ * Release locks previously acquired by
ieee80211_txq_schedule_end().
Check
+ * and remove the empty txq from rb-tree.
*/
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8
ac)
__releases(txq_lock);
@@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
ieee80211_hw
*hw, struct ieee80211_txq *txq)
__acquires(txq_lock) __releases(txq_lock);
/**
+ * ieee80211_txqs_check - Check txqs waiting for removal
+ *
+ * @tmr: pointer as obtained from local
+ *
+ */
+void ieee80211_txqs_check(struct timer_list *tmr);
+
+/**
* ieee80211_txq_may_transmit - check whether TXQ is allowed
to
transmit
*
* This function is used to check whether given txq is
allowed
to
transmit by
diff --git a/net/mac80211/ieee80211_i.h
b/net/mac80211/ieee80211_i.h
index a4556f9..49aa143e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -847,6 +847,7 @@ struct txq_info {
struct codel_stats cstats;
struct sk_buff_head frags;
struct rb_node schedule_order;
+ struct list_head candidate;
unsigned long flags;
/* keep last! */
@@ -1145,6 +1146,8 @@ struct ieee80211_local {
u64 airtime_v_t[IEEE80211_NUM_ACS];
u64 airtime_weight_sum[IEEE80211_NUM_ACS];
+ struct list_head remove_list[IEEE80211_NUM_ACS];
+ struct timer_list remove_timer;
u16 airtime_flags;
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index e9ffa8e..78fe24a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -667,10 +667,15 @@ struct ieee80211_hw
*ieee80211_alloc_hw_nm(size_t priv_data_len,
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
local->active_txqs[i] = RB_ROOT_CACHED;
+ INIT_LIST_HEAD(>remove_list[i]);
spin_lock_init(>active_txq_lock[i]);
}
local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
+ timer_setup(>remove_timer, ieee80211_txqs_check, 0);
+ mod_timer(>remove_timer,
+ jiffies +
msecs_to_jiffies(IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS));
+
INIT_LIST_HEAD(>chanctx_list);
mutex_init(>chanctx_mtx);
@@ -1305,6 +1310,7 @@ void ieee80211_unregister_hw(struct
ieee80211_hw
*hw)
tasklet_kill(>tx_pending_tasklet);
tasklet_kill(>tasklet);
+ del_timer_sync(>remove_timer);
#ifdef CONFIG_INET
unregister_inetaddr_notifier(>ifa_notifier);
#endif
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index d00baaa..42ca010 100644
---
Re: [PATCH v2] ath10k: add fw coredump for sdio when firmware assert
Wen Gong writes:
> When firmware assert, it need coredump to analyze, this patch will
> collect the register and memory info for sdio chip.
>
> The coredump configuration is different between PCIE and SDIO for
> the same reversion, so this patch add bus type to distinguish PCIE
> and SDIO chip for coredump.
>
> Tested with QCA6174 SDIO with firmware
> WLAN.RMH.4.4.1-7-QCARMSWP-1.
>
> Signed-off-by: Wen Gong
[...]
> +void ath10k_sdio_fw_crashed_dump(struct ath10k *ar)
> +{
> + struct ath10k_fw_crash_data *crash_data;
> + char guid[UUID_STRING_LEN + 1];
> + u32 fast_dump = 0;
> +
> + ath10k_err(ar, "begin fw dump\n", guid);
> +
> + ath10k_sdio_check_fw_reg(ar, _dump);
> +
> + if (fast_dump)
> + ar->bmi.done_sent = false;
After looking more closely, the ar->bmi.done_set checks in bmi.c does
not look necessary to me, I have never seen that warning. I would remove
those and the done_sent field altogether from ath10k to make the code
cleaner and I avoid hacks like above. This should be done in a separate
patch, of course.
--
Kalle Valo
Re: [PATCH v2] ath10k: add fw coredump for sdio when firmware assert
Wen Gong writes:
> When firmware assert, it need coredump to analyze, this patch will
> collect the register and memory info for sdio chip.
>
> The coredump configuration is different between PCIE and SDIO for
> the same reversion, so this patch add bus type to distinguish PCIE
> and SDIO chip for coredump.
>
> Tested with QCA6174 SDIO with firmware
> WLAN.RMH.4.4.1-7-QCARMSWP-1.
>
> Signed-off-by: Wen Gong
[...]
> +static int ath10k_sdio_read_mem(struct ath10k *ar, u32 address, void *buf,
> + u32 buf_len)
> +{
> + u32 val;
> + int i, ret;
> +
> + for (i = 0; i < buf_len; i += 4) {
> + ret = ath10k_sdio_hif_diag_read32(ar, address + i, );
> + if (ret) {
> + ath10k_warn(ar, "unable to read mem %d value\n",
> address + i);
> + break;
> + }
> + memcpy(buf + i, , 4);
> + }
> +
> + return ret;
> +}
What's wrong with ath10k_sdio_hif_diag_read()? AFAICS this whole
function duplicates just what it does.
--
Kalle Valo
Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
Yibo Zhao writes:
> On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
> On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
>> Yibo Zhao writes:
>>
>>> On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
> In a loop txqs dequeue scenario, if the first txq in the rbtree
> gets
> removed from rbtree immediately in the ieee80211_return_txq(),
> the
> loop will break soon in the ieee80211_next_txq() due to
> schedule_pos
> not leading to the second txq in the rbtree. Thus, defering the
> removal right before the end of this schedule round.
>
> Co-developed-by: Yibo Zhao
> Signed-off-by: Yibo Zhao
> Signed-off-by: Toke Høiland-Jørgensen
I didn't write this patch, so please don't use my sign-off. I'll
add
ack or review tags as appropriate in reply; but a few comments
first:
> ---
> include/net/mac80211.h | 16 ++--
> net/mac80211/ieee80211_i.h | 3 +++
> net/mac80211/main.c| 6 +
> net/mac80211/tx.c | 63
> +++---
> 4 files changed, 83 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index ac2ed8e..ba5a345 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
>
> #define IEEE80211_MAX_TX_RETRY 31
>
> +#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
> +
> static inline void ieee80211_rate_set_vht(struct
> ieee80211_tx_rate
> *rate,
> u8 mcs, u8 nss)
> {
> @@ -6232,7 +6234,8 @@ struct sk_buff
> *ieee80211_tx_dequeue(struct
> ieee80211_hw *hw,
> * @ac: AC number to return packets from.
> *
> * Should only be called between calls to
> ieee80211_txq_schedule_start()
> - * and ieee80211_txq_schedule_end().
> + * and ieee80211_txq_schedule_end(). If the txq is empty, it
> will
> be
> added
> + * to a remove list and get removed later.
> * Returns the next txq if successful, %NULL if no queue is
> eligible.
> If a txq
> * is returned, it should be returned with
> ieee80211_return_txq()
> after the
> * driver has finished scheduling it.
> @@ -6268,7 +6271,8 @@ void ieee80211_txq_schedule_start(struct
> ieee80211_hw *hw, u8 ac)
> * @hw: pointer as obtained from ieee80211_alloc_hw()
> * @ac: AC number to acquire locks for
> *
> - * Release locks previously acquired by
> ieee80211_txq_schedule_end().
> + * Release locks previously acquired by
> ieee80211_txq_schedule_end().
> Check
> + * and remove the empty txq from rb-tree.
> */
> void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
> __releases(txq_lock);
> @@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
> ieee80211_hw
> *hw, struct ieee80211_txq *txq)
> __acquires(txq_lock) __releases(txq_lock);
>
> /**
> + * ieee80211_txqs_check - Check txqs waiting for removal
> + *
> + * @tmr: pointer as obtained from local
> + *
> + */
> +void ieee80211_txqs_check(struct timer_list *tmr);
> +
> +/**
> * ieee80211_txq_may_transmit - check whether TXQ is allowed to
> transmit
> *
> * This function is used to check whether given txq is allowed
> to
> transmit by
> diff --git a/net/mac80211/ieee80211_i.h
> b/net/mac80211/ieee80211_i.h
> index a4556f9..49aa143e 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -847,6 +847,7 @@ struct txq_info {
> struct codel_stats cstats;
> struct sk_buff_head frags;
> struct rb_node schedule_order;
> + struct list_head candidate;
> unsigned long flags;
>
> /* keep last! */
> @@ -1145,6 +1146,8 @@ struct ieee80211_local {
> u64 airtime_v_t[IEEE80211_NUM_ACS];
> u64 airtime_weight_sum[IEEE80211_NUM_ACS];
>
> + struct list_head remove_list[IEEE80211_NUM_ACS];
> + struct timer_list remove_timer;
> u16 airtime_flags;
>
> const struct ieee80211_ops
Re: [PATCH v2] ath10k: add fw coredump for sdio when firmware assert
Wen Gong writes:
> When firmware assert, it need coredump to analyze, this patch will
> collect the register and memory info for sdio chip.
>
> The coredump configuration is different between PCIE and SDIO for
> the same reversion, so this patch add bus type to distinguish PCIE
> and SDIO chip for coredump.
>
> Tested with QCA6174 SDIO with firmware
> WLAN.RMH.4.4.1-7-QCARMSWP-1.
>
> Signed-off-by: Wen Gong
[...]
> +void ath10k_sdio_check_fw_reg(struct ath10k *ar, u32 *fast_dump)
> +{
> + int ret = 0;
> + u32 param;
> +
> + ret = ath10k_sdio_read_host_interest_value(ar,
> HI_ITEM(hi_option_flag2), );
> +
> + *fast_dump = ((param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW)
> + == HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW);
The commit log tells nothing about fast, it should always document the
design decisions. Why this fast dump, what's the benefit? Why not always
use the fast dump and forget the slow dump (or vice versa)? There needs
to be really good reasons to have all this complexity to support both
slow and fast dumps.
> + ath10k_err(ar, "check fw reg : %x\n", param);
> +}
This is a debug message, not an error. And debug messages should use
format "sdio hi_option_flag2 %x\n".
--
Kalle Valo
Re: [PATCH v2] ath10k: add fw coredump for sdio when firmware assert
Wen Gong writes:
> When firmware assert, it need coredump to analyze, this patch will
> collect the register and memory info for sdio chip.
>
> The coredump configuration is different between PCIE and SDIO for
> the same reversion, so this patch add bus type to distinguish PCIE
> and SDIO chip for coredump.
>
> Tested with QCA6174 SDIO with firmware
> WLAN.RMH.4.4.1-7-QCARMSWP-1.
>
> Signed-off-by: Wen Gong
I think this patch should be split into two, the first patch preparing
coredump layouts to support different bus types and the second patch
adding coredump support for SDIO.
> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -11,6 +11,7 @@
> #include "targaddrs.h"
>
> enum ath10k_bus {
> + ATH10K_BUS_ANY,
> ATH10K_BUS_PCI,
> ATH10K_BUS_AHB,
> ATH10K_BUS_SDIO,
I don't think we need add this type ANY. From a quick look QCA4019 is
AHB and all the rest are PCI, right? So in the first patch you only need
to add correct bus type for each entry and everything should work as
before.
--
Kalle Valo
Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree
On 2019-09-20 17:15, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-19 18:37, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 19:23, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
On 2019-09-18 05:10, Toke Høiland-Jørgensen wrote:
Yibo Zhao writes:
In a loop txqs dequeue scenario, if the first txq in the rbtree
gets
removed from rbtree immediately in the ieee80211_return_txq(),
the
loop will break soon in the ieee80211_next_txq() due to
schedule_pos
not leading to the second txq in the rbtree. Thus, defering the
removal right before the end of this schedule round.
Co-developed-by: Yibo Zhao
Signed-off-by: Yibo Zhao
Signed-off-by: Toke Høiland-Jørgensen
I didn't write this patch, so please don't use my sign-off. I'll
add
ack or review tags as appropriate in reply; but a few comments
first:
---
include/net/mac80211.h | 16 ++--
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/main.c| 6 +
net/mac80211/tx.c | 63
+++---
4 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ac2ed8e..ba5a345 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
#define IEEE80211_MAX_TX_RETRY 31
+#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
+
static inline void ieee80211_rate_set_vht(struct
ieee80211_tx_rate
*rate,
u8 mcs, u8 nss)
{
@@ -6232,7 +6234,8 @@ struct sk_buff
*ieee80211_tx_dequeue(struct
ieee80211_hw *hw,
* @ac: AC number to return packets from.
*
* Should only be called between calls to
ieee80211_txq_schedule_start()
- * and ieee80211_txq_schedule_end().
+ * and ieee80211_txq_schedule_end(). If the txq is empty, it
will
be
added
+ * to a remove list and get removed later.
* Returns the next txq if successful, %NULL if no queue is
eligible.
If a txq
* is returned, it should be returned with
ieee80211_return_txq()
after the
* driver has finished scheduling it.
@@ -6268,7 +6271,8 @@ void ieee80211_txq_schedule_start(struct
ieee80211_hw *hw, u8 ac)
* @hw: pointer as obtained from ieee80211_alloc_hw()
* @ac: AC number to acquire locks for
*
- * Release locks previously acquired by
ieee80211_txq_schedule_end().
+ * Release locks previously acquired by
ieee80211_txq_schedule_end().
Check
+ * and remove the empty txq from rb-tree.
*/
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
__releases(txq_lock);
@@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct
ieee80211_hw
*hw, struct ieee80211_txq *txq)
__acquires(txq_lock) __releases(txq_lock);
/**
+ * ieee80211_txqs_check - Check txqs waiting for removal
+ *
+ * @tmr: pointer as obtained from local
+ *
+ */
+void ieee80211_txqs_check(struct timer_list *tmr);
+
+/**
* ieee80211_txq_may_transmit - check whether TXQ is allowed to
transmit
*
* This function is used to check whether given txq is allowed
to
transmit by
diff --git a/net/mac80211/ieee80211_i.h
b/net/mac80211/ieee80211_i.h
index a4556f9..49aa143e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -847,6 +847,7 @@ struct txq_info {
struct codel_stats cstats;
struct sk_buff_head frags;
struct rb_node schedule_order;
+ struct list_head candidate;
unsigned long flags;
/* keep last! */
@@ -1145,6 +1146,8 @@ struct ieee80211_local {
u64 airtime_v_t[IEEE80211_NUM_ACS];
u64 airtime_weight_sum[IEEE80211_NUM_ACS];
+ struct list_head remove_list[IEEE80211_NUM_ACS];
+ struct timer_list remove_timer;
u16 airtime_flags;
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index e9ffa8e..78fe24a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -667,10 +667,15 @@ struct ieee80211_hw
*ieee80211_alloc_hw_nm(size_t priv_data_len,
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
local->active_txqs[i] = RB_ROOT_CACHED;
+ INIT_LIST_HEAD(>remove_list[i]);
spin_lock_init(>active_txq_lock[i]);
}
local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
+ timer_setup(>remove_timer, ieee80211_txqs_check, 0);
+ mod_timer(>remove_timer,
+ jiffies +
msecs_to_jiffies(IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS));
+
INIT_LIST_HEAD(>chanctx_list);
mutex_init(>chanctx_mtx);
@@ -1305,6 +1310,7 @@ void ieee80211_unregister_hw(struct
ieee80211_hw
*hw)
tasklet_kill(>tx_pending_tasklet);
tasklet_kill(>tasklet);
+ del_timer_sync(>remove_timer);
#ifdef CONFIG_INET
unregister_inetaddr_notifier(>ifa_notifier);
#endif
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index d00baaa..42ca010 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1450,6 +1450,7 @@ void
Re: [PATCH v4] ath10k: Enable MSA region dump support for WCN3990
Govind Singh wrote: > MSA memory region caries the hw descriptors information. > Dump MSA region in core dump as this is very helpful in debugging > hw issues. > > Testing: Tested on WCN3990 HW > Tested FW: WLAN.HL.3.1-00959-QCAHLSWMTPLZ-1 > > Signed-off-by: Govind Singh > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 3f14b73c3843 ath10k: Enable MSA region dump support for WCN3990 -- https://patchwork.kernel.org/patch/11036163/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v4] ath10k: Enable MSA region dump support for WCN3990
Govind Singh wrote: > MSA memory region caries the hw descriptors information. > Dump MSA region in core dump as this is very helpful in debugging > hw issues. > > Testing: Tested on WCN3990 HW > Tested FW: WLAN.HL.3.1-00959-QCAHLSWMTPLZ-1 > > Signed-off-by: Govind Singh > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 3f14b73c3843 ath10k: Enable MSA region dump support for WCN3990 -- https://patchwork.kernel.org/patch/11036163/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v3 1/2] dt: bindings: add dt entry for XO calibration support
Govind Singh wrote: > Add dt binding to get xo calibration data support for wifi rf clock. > > Signed-off-by: Govind Singh > Reviewed-by: Rob Herring > Signed-off-by: Kalle Valo 2 patches applied to ath-next branch of ath.git, thanks. 892022e108dd dt: bindings: ath10k: add dt entry for XO calibration support 75f545e85744 ath10k: Add xo calibration support for wifi rf clock -- https://patchwork.kernel.org/patch/10879475/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v3 1/2] dt: bindings: add dt entry for XO calibration support
Govind Singh wrote: > Add dt binding to get xo calibration data support for wifi rf clock. > > Signed-off-by: Govind Singh > Reviewed-by: Rob Herring > Signed-off-by: Kalle Valo 2 patches applied to ath-next branch of ath.git, thanks. 892022e108dd dt: bindings: ath10k: add dt entry for XO calibration support 75f545e85744 ath10k: Add xo calibration support for wifi rf clock -- https://patchwork.kernel.org/patch/10879475/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
