This patch series includes patches which improve TX throughputs.
Marc Yang (1):
mwifiex: avoid queue_work while work is ongoing
Zhaoyang Liu (3):
mwifiex: get rid of BA setup helper functions
mwifiex: remove_bss_prio_lock
mwifiex: preprocess packets from TX queue
drivers/net/wireless/mwifiex/11n.c | 18 +++-
drivers/net/wireless/mwifiex/11n.h | 32 -------
drivers/net/wireless/mwifiex/11n_aggr.c | 14 ++-
drivers/net/wireless/mwifiex/11n_rxreorder.c | 7 +-
drivers/net/wireless/mwifiex/cfg80211.c | 33 +++++++
drivers/net/wireless/mwifiex/decl.h | 2 +
drivers/net/wireless/mwifiex/init.c | 5 ++
drivers/net/wireless/mwifiex/main.c | 71 ++++++++++++---
drivers/net/wireless/mwifiex/main.h | 21 +++--
drivers/net/wireless/mwifiex/pcie.c | 2 +-
drivers/net/wireless/mwifiex/txrx.c | 126 +++++++++++++++++++++++++++
drivers/net/wireless/mwifiex/usb.c | 4 +-
drivers/net/wireless/mwifiex/wmm.c | 49 ++++++-----
drivers/net/wireless/mwifiex/wmm.h | 2 +
14 files changed, 305 insertions(+), 81 deletions(-)
--
1.8.1.4
From: Zhaoyang Liu <[email protected]>
This patch removes BA setup helper routines
mwifiex_is_bastream_setup and mwifiex_is_amsdu_in_ampdu_allowed.
Current code will use two functions to check bastream setup and
amsdu in ampdu. This patch change these functions to flags, thus
avoiding redundant spin_lock check while dequeuing TX packets.
Signed-off-by: Zhaoyang Liu <[email protected]>
Signed-off-by: Shengzhen Li <[email protected]>
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/11n.c | 18 +++++++++++++++-
drivers/net/wireless/mwifiex/11n.h | 32 ----------------------------
drivers/net/wireless/mwifiex/11n_rxreorder.c | 7 +++++-
drivers/net/wireless/mwifiex/main.h | 13 ++++++-----
drivers/net/wireless/mwifiex/wmm.c | 18 +++++++++-------
drivers/net/wireless/mwifiex/wmm.h | 2 ++
6 files changed, 43 insertions(+), 47 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/11n.c b/drivers/net/wireless/mwifiex/11n.c
index 543148d..433bd68 100644
--- a/drivers/net/wireless/mwifiex/11n.c
+++ b/drivers/net/wireless/mwifiex/11n.c
@@ -159,6 +159,7 @@ int mwifiex_ret_11n_addba_req(struct mwifiex_private *priv,
int tid;
struct host_cmd_ds_11n_addba_rsp *add_ba_rsp = &resp->params.add_ba_rsp;
struct mwifiex_tx_ba_stream_tbl *tx_ba_tbl;
+ struct mwifiex_ra_list_tbl *ra_list;
u16 block_ack_param_set = le16_to_cpu(add_ba_rsp->block_ack_param_set);
add_ba_rsp->ssn = cpu_to_le16((le16_to_cpu(add_ba_rsp->ssn))
@@ -166,7 +167,13 @@ int mwifiex_ret_11n_addba_req(struct mwifiex_private *priv,
tid = (block_ack_param_set & IEEE80211_ADDBA_PARAM_TID_MASK)
>> BLOCKACKPARAM_TID_POS;
+ ra_list = mwifiex_wmm_get_ralist_node(priv, tid, add_ba_rsp->
+ peer_mac_addr);
if (le16_to_cpu(add_ba_rsp->status_code) != BA_RESULT_SUCCESS) {
+ if (ra_list) {
+ ra_list->ba_status = BA_SETUP_NONE;
+ ra_list->amsdu_in_ampdu = false;
+ }
mwifiex_del_ba_tbl(priv, tid, add_ba_rsp->peer_mac_addr,
TYPE_DELBA_SENT, true);
if (add_ba_rsp->add_rsp_result != BA_RESULT_TIMEOUT)
@@ -185,6 +192,10 @@ int mwifiex_ret_11n_addba_req(struct mwifiex_private *priv,
tx_ba_tbl->amsdu = true;
else
tx_ba_tbl->amsdu = false;
+ if (ra_list) {
+ ra_list->amsdu_in_ampdu = tx_ba_tbl->amsdu;
+ ra_list->ba_status = BA_SETUP_COMPLETE;
+ }
} else {
dev_err(priv->adapter->dev, "BA stream not created\n");
}
@@ -515,6 +526,7 @@ void mwifiex_create_ba_tbl(struct mwifiex_private *priv, u8 *ra, int tid,
enum mwifiex_ba_status ba_status)
{
struct mwifiex_tx_ba_stream_tbl *new_node;
+ struct mwifiex_ra_list_tbl *ra_list;
unsigned long flags;
if (!mwifiex_get_ba_tbl(priv, tid, ra)) {
@@ -522,7 +534,11 @@ void mwifiex_create_ba_tbl(struct mwifiex_private *priv, u8 *ra, int tid,
GFP_ATOMIC);
if (!new_node)
return;
-
+ ra_list = mwifiex_wmm_get_ralist_node(priv, tid, ra);
+ if (ra_list) {
+ ra_list->ba_status = ba_status;
+ ra_list->amsdu_in_ampdu = false;
+ }
INIT_LIST_HEAD(&new_node->list);
new_node->tid = tid;
diff --git a/drivers/net/wireless/mwifiex/11n.h b/drivers/net/wireless/mwifiex/11n.h
index 8e2e394..afdd58a 100644
--- a/drivers/net/wireless/mwifiex/11n.h
+++ b/drivers/net/wireless/mwifiex/11n.h
@@ -77,22 +77,6 @@ mwifiex_is_station_ampdu_allowed(struct mwifiex_private *priv,
return (node->ampdu_sta[tid] != BA_STREAM_NOT_ALLOWED) ? true : false;
}
-/* This function checks whether AMSDU is allowed for BA stream. */
-static inline u8
-mwifiex_is_amsdu_in_ampdu_allowed(struct mwifiex_private *priv,
- struct mwifiex_ra_list_tbl *ptr, int tid)
-{
- struct mwifiex_tx_ba_stream_tbl *tx_tbl;
-
- if (is_broadcast_ether_addr(ptr->ra))
- return false;
- tx_tbl = mwifiex_get_ba_tbl(priv, tid, ptr->ra);
- if (tx_tbl)
- return tx_tbl->amsdu;
-
- return false;
-}
-
/* This function checks whether AMPDU is allowed or not for a particular TID. */
static inline u8
mwifiex_is_ampdu_allowed(struct mwifiex_private *priv,
@@ -182,22 +166,6 @@ mwifiex_find_stream_to_delete(struct mwifiex_private *priv, int ptr_tid,
}
/*
- * This function checks whether BA stream is set up or not.
- */
-static inline int
-mwifiex_is_ba_stream_setup(struct mwifiex_private *priv,
- struct mwifiex_ra_list_tbl *ptr, int tid)
-{
- struct mwifiex_tx_ba_stream_tbl *tx_tbl;
-
- tx_tbl = mwifiex_get_ba_tbl(priv, tid, ptr->ra);
- if (tx_tbl && IS_BASTREAM_SETUP(tx_tbl))
- return true;
-
- return false;
-}
-
-/*
* This function checks whether associated station is 11n enabled
*/
static inline int mwifiex_is_sta_11n_enabled(struct mwifiex_private *priv,
diff --git a/drivers/net/wireless/mwifiex/11n_rxreorder.c b/drivers/net/wireless/mwifiex/11n_rxreorder.c
index a2e8817..f75f8ac 100644
--- a/drivers/net/wireless/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/mwifiex/11n_rxreorder.c
@@ -659,6 +659,7 @@ mwifiex_del_ba_tbl(struct mwifiex_private *priv, int tid, u8 *peer_mac,
{
struct mwifiex_rx_reorder_tbl *tbl;
struct mwifiex_tx_ba_stream_tbl *ptx_tbl;
+ struct mwifiex_ra_list_tbl *ra_list;
u8 cleanup_rx_reorder_tbl;
unsigned long flags;
@@ -686,7 +687,11 @@ mwifiex_del_ba_tbl(struct mwifiex_private *priv, int tid, u8 *peer_mac,
"event: TID, RA not found in table\n");
return;
}
-
+ ra_list = mwifiex_wmm_get_ralist_node(priv, tid, peer_mac);
+ if (ra_list) {
+ ra_list->amsdu_in_ampdu = false;
+ ra_list->ba_status = BA_SETUP_NONE;
+ }
spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags);
mwifiex_11n_delete_tx_ba_stream_tbl_entry(priv, ptx_tbl);
spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags);
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index 801a23b..b47bffa 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -210,6 +210,12 @@ struct mwifiex_tx_aggr {
u8 amsdu;
};
+enum mwifiex_ba_status {
+ BA_SETUP_NONE = 0,
+ BA_SETUP_INPROGRESS,
+ BA_SETUP_COMPLETE
+};
+
struct mwifiex_ra_list_tbl {
struct list_head list;
struct sk_buff_head skb_head;
@@ -218,6 +224,8 @@ struct mwifiex_ra_list_tbl {
u16 max_amsdu;
u16 ba_pkt_count;
u8 ba_packet_thr;
+ enum mwifiex_ba_status ba_status;
+ u8 amsdu_in_ampdu;
u16 total_pkt_count;
bool tdls_link;
};
@@ -601,11 +609,6 @@ struct mwifiex_private {
struct mwifiex_11h_intf_state state_11h;
};
-enum mwifiex_ba_status {
- BA_SETUP_NONE = 0,
- BA_SETUP_INPROGRESS,
- BA_SETUP_COMPLETE
-};
struct mwifiex_tx_ba_stream_tbl {
struct list_head list;
diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c
index ef717ac..ba75a45 100644
--- a/drivers/net/wireless/mwifiex/wmm.c
+++ b/drivers/net/wireless/mwifiex/wmm.c
@@ -157,6 +157,8 @@ void mwifiex_ralist_add(struct mwifiex_private *priv, const u8 *ra)
ra_list->is_11n_enabled = 0;
ra_list->tdls_link = false;
+ ra_list->ba_status = BA_SETUP_NONE;
+ ra_list->amsdu_in_ampdu = false;
if (!mwifiex_queuing_ra_based(priv)) {
if (mwifiex_get_tdls_link_status(priv, ra) ==
TDLS_SETUP_COMPLETE) {
@@ -574,7 +576,7 @@ mwifiex_clean_txrx(struct mwifiex_private *priv)
* This function retrieves a particular RA list node, matching with the
* given TID and RA address.
*/
-static struct mwifiex_ra_list_tbl *
+struct mwifiex_ra_list_tbl *
mwifiex_wmm_get_ralist_node(struct mwifiex_private *priv, u8 tid,
const u8 *ra_addr)
{
@@ -1276,14 +1278,14 @@ mwifiex_dequeue_tx_packet(struct mwifiex_adapter *adapter)
}
if (!ptr->is_11n_enabled ||
- mwifiex_is_ba_stream_setup(priv, ptr, tid) ||
- priv->wps.session_enable) {
+ ptr->ba_status ||
+ priv->wps.session_enable) {
if (ptr->is_11n_enabled &&
- mwifiex_is_ba_stream_setup(priv, ptr, tid) &&
- mwifiex_is_amsdu_in_ampdu_allowed(priv, ptr, tid) &&
- mwifiex_is_amsdu_allowed(priv, tid) &&
- mwifiex_is_11n_aggragation_possible(priv, ptr,
- adapter->tx_buf_size))
+ ptr->ba_status &&
+ ptr->amsdu_in_ampdu &&
+ mwifiex_is_amsdu_allowed(priv, tid) &&
+ mwifiex_is_11n_aggragation_possible(priv, ptr,
+ adapter->tx_buf_size))
mwifiex_11n_aggregate_pkt(priv, ptr, ptr_index, flags);
/* ra_list_spinlock has been freed in
* mwifiex_11n_aggregate_pkt()
diff --git a/drivers/net/wireless/mwifiex/wmm.h b/drivers/net/wireless/mwifiex/wmm.h
index 569bd73..48ece0b 100644
--- a/drivers/net/wireless/mwifiex/wmm.h
+++ b/drivers/net/wireless/mwifiex/wmm.h
@@ -127,4 +127,6 @@ mwifiex_wmm_get_queue_raptr(struct mwifiex_private *priv, u8 tid,
const u8 *ra_addr);
u8 mwifiex_wmm_downgrade_tid(struct mwifiex_private *priv, u32 tid);
+struct mwifiex_ra_list_tbl *mwifiex_wmm_get_ralist_node(struct mwifiex_private
+ *priv, u8 tid, const u8 *ra_addr);
#endif /* !_MWIFIEX_WMM_H_ */
--
1.8.1.4
From: Zhaoyang Liu <[email protected]>
This patch does away with spinlock in
mwifiex_wmm_get_highest_priolist_ptr in order to improve TP.
Signed-off-by: Zhaoyang Liu <[email protected]>
Signed-off-by: Shengzhen Li <[email protected]>
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/cfg80211.c | 33 +++++++++++++++++++++++++++++++++
drivers/net/wireless/mwifiex/main.c | 2 +-
drivers/net/wireless/mwifiex/main.h | 1 +
drivers/net/wireless/mwifiex/wmm.c | 11 ++---------
4 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index 5f3c1d3..fb55092 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -717,6 +717,9 @@ mwifiex_cfg80211_init_p2p_go(struct mwifiex_private *priv)
static int mwifiex_deinit_priv_params(struct mwifiex_private *priv)
{
+ struct mwifiex_adapter *adapter = priv->adapter;
+ unsigned long flags;
+
priv->mgmt_frame_mask = 0;
if (mwifiex_send_cmd(priv, HostCmd_CMD_MGMT_FRAME_REG,
HostCmd_ACT_GEN_SET, 0,
@@ -727,6 +730,25 @@ static int mwifiex_deinit_priv_params(struct mwifiex_private *priv)
}
mwifiex_deauthenticate(priv, NULL);
+
+ spin_lock_irqsave(&adapter->main_proc_lock, flags);
+ adapter->main_locked = true;
+ if (adapter->mwifiex_processing) {
+ spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
+ flush_workqueue(adapter->workqueue);
+ } else {
+ spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
+ }
+
+ spin_lock_irqsave(&adapter->rx_proc_lock, flags);
+ adapter->rx_locked = true;
+ if (adapter->rx_processing) {
+ spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
+ flush_workqueue(adapter->rx_workqueue);
+ } else {
+ spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
+ }
+
mwifiex_free_priv(priv);
priv->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
@@ -740,6 +762,9 @@ mwifiex_init_new_priv_params(struct mwifiex_private *priv,
struct net_device *dev,
enum nl80211_iftype type)
{
+ struct mwifiex_adapter *adapter = priv->adapter;
+ unsigned long flags;
+
mwifiex_init_priv(priv);
priv->bss_mode = type;
@@ -770,6 +795,14 @@ mwifiex_init_new_priv_params(struct mwifiex_private *priv,
return -EOPNOTSUPP;
}
+ spin_lock_irqsave(&adapter->main_proc_lock, flags);
+ adapter->main_locked = false;
+ spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
+
+ spin_lock_irqsave(&adapter->rx_proc_lock, flags);
+ adapter->rx_locked = false;
+ spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
+
return 0;
}
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 447eb6f..46e1789 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -217,7 +217,7 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
spin_lock_irqsave(&adapter->main_proc_lock, flags);
/* Check if already processing */
- if (adapter->mwifiex_processing) {
+ if (adapter->mwifiex_processing || adapter->main_locked) {
adapter->more_task_flag = true;
spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
goto exit_main_proc;
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index b47bffa..cc6a623 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -773,6 +773,7 @@ struct mwifiex_adapter {
bool rx_work_enabled;
bool rx_processing;
bool delay_main_work;
+ bool main_locked;
bool rx_locked;
struct mwifiex_bss_prio_tbl bss_prio_tbl[MWIFIEX_MAX_BSS_NUM];
/* spin lock for init/shutdown */
diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c
index ba75a45..ce747f7 100644
--- a/drivers/net/wireless/mwifiex/wmm.c
+++ b/drivers/net/wireless/mwifiex/wmm.c
@@ -944,14 +944,11 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter,
struct mwifiex_ra_list_tbl *ptr;
struct mwifiex_tid_tbl *tid_ptr;
atomic_t *hqp;
- unsigned long flags_bss, flags_ra;
+ unsigned long flags_ra;
int i, j;
/* check the BSS with highest priority first */
for (j = adapter->priv_num - 1; j >= 0; --j) {
- spin_lock_irqsave(&adapter->bss_prio_tbl[j].bss_prio_lock,
- flags_bss);
-
/* iterate over BSS with the equal priority */
list_for_each_entry(adapter->bss_prio_tbl[j].bss_prio_cur,
&adapter->bss_prio_tbl[j].bss_prio_head,
@@ -987,19 +984,15 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter,
}
}
- spin_unlock_irqrestore(&adapter->bss_prio_tbl[j].bss_prio_lock,
- flags_bss);
}
return NULL;
found:
- /* holds bss_prio_lock / ra_list_spinlock */
+ /* holds ra_list_spinlock */
if (atomic_read(hqp) > i)
atomic_set(hqp, i);
spin_unlock_irqrestore(&priv_tmp->wmm.ra_list_spinlock, flags_ra);
- spin_unlock_irqrestore(&adapter->bss_prio_tbl[j].bss_prio_lock,
- flags_bss);
*priv = priv_tmp;
*tid = tos_to_tid[i];
--
1.8.1.4
From: Marc Yang <[email protected]>
Current code does not check whether main_work_queue or
rx_work_queue is running when preparing to do queue_work,
this code fix add check before calling queue_work, reducing
unnecessary queue_work switch.
This change instead sets more_task flag to ensure we run main_process
superloop once again.
Signed-off-by: Marc Yang <[email protected]>
Signed-off-by: Zhaoyang Liu <[email protected]>
Signed-off-by: Shengzhen Li <[email protected]>
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/main.c | 38 +++++++++++++++++++++++++++++++------
drivers/net/wireless/mwifiex/main.h | 1 +
drivers/net/wireless/mwifiex/pcie.c | 2 +-
drivers/net/wireless/mwifiex/usb.c | 4 ++--
4 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 74488ab..447eb6f 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -131,6 +131,34 @@ static int mwifiex_unregister(struct mwifiex_adapter *adapter)
return 0;
}
+void mwifiex_queue_main_work(struct mwifiex_adapter *adapter)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&adapter->main_proc_lock, flags);
+ if (adapter->mwifiex_processing) {
+ adapter->more_task_flag = true;
+ spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
+ } else {
+ spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
+ queue_work(adapter->workqueue, &adapter->main_work);
+ }
+}
+EXPORT_SYMBOL_GPL(mwifiex_queue_main_work);
+
+static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&adapter->rx_proc_lock, flags);
+ if (adapter->rx_processing) {
+ spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
+ } else {
+ spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
+ queue_work(adapter->rx_workqueue, &adapter->rx_work);
+ }
+}
+
static int mwifiex_process_rx(struct mwifiex_adapter *adapter)
{
unsigned long flags;
@@ -154,7 +182,7 @@ static int mwifiex_process_rx(struct mwifiex_adapter *adapter)
if (adapter->if_ops.submit_rem_rx_urbs)
adapter->if_ops.submit_rem_rx_urbs(adapter);
adapter->delay_main_work = false;
- queue_work(adapter->workqueue, &adapter->main_work);
+ mwifiex_queue_main_work(adapter);
}
mwifiex_handle_rx_packet(adapter, skb);
}
@@ -214,9 +242,7 @@ process_start:
if (atomic_read(&adapter->rx_pending) >= HIGH_RX_PENDING &&
adapter->iface_type != MWIFIEX_USB) {
adapter->delay_main_work = true;
- if (!adapter->rx_processing)
- queue_work(adapter->rx_workqueue,
- &adapter->rx_work);
+ mwifiex_queue_rx_work(adapter);
break;
}
@@ -229,7 +255,7 @@ process_start:
}
if (adapter->rx_work_enabled && adapter->data_received)
- queue_work(adapter->rx_workqueue, &adapter->rx_work);
+ mwifiex_queue_rx_work(adapter);
/* Need to wake up the card ? */
if ((adapter->ps_state == PS_STATE_SLEEP) &&
@@ -606,7 +632,7 @@ int mwifiex_queue_tx_pkt(struct mwifiex_private *priv, struct sk_buff *skb)
atomic_inc(&priv->adapter->tx_pending);
mwifiex_wmm_add_buf_txqueue(priv, skb);
- queue_work(priv->adapter->workqueue, &priv->adapter->main_work);
+ mwifiex_queue_main_work(priv->adapter);
return 0;
}
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index 16be45e..801a23b 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -1422,6 +1422,7 @@ u8 mwifiex_adjust_data_rate(struct mwifiex_private *priv,
void mwifiex_dump_drv_info(struct mwifiex_adapter *adapter);
void *mwifiex_alloc_rx_buf(int rx_len, gfp_t flags);
+void mwifiex_queue_main_work(struct mwifiex_adapter *adapter);
#ifdef CONFIG_DEBUG_FS
void mwifiex_debugfs_init(void);
diff --git a/drivers/net/wireless/mwifiex/pcie.c b/drivers/net/wireless/mwifiex/pcie.c
index 4b463c3..c0ff33e 100644
--- a/drivers/net/wireless/mwifiex/pcie.c
+++ b/drivers/net/wireless/mwifiex/pcie.c
@@ -2101,7 +2101,7 @@ static irqreturn_t mwifiex_pcie_interrupt(int irq, void *context)
goto exit;
mwifiex_interrupt_status(adapter);
- queue_work(adapter->workqueue, &adapter->main_work);
+ mwifiex_queue_main_work(adapter);
exit:
return IRQ_HANDLED;
diff --git a/drivers/net/wireless/mwifiex/usb.c b/drivers/net/wireless/mwifiex/usb.c
index 2238730..3adf1f6 100644
--- a/drivers/net/wireless/mwifiex/usb.c
+++ b/drivers/net/wireless/mwifiex/usb.c
@@ -193,7 +193,7 @@ static void mwifiex_usb_rx_complete(struct urb *urb)
dev_dbg(adapter->dev, "info: recv_length=%d, status=%d\n",
recv_length, status);
if (status == -EINPROGRESS) {
- queue_work(adapter->workqueue, &adapter->main_work);
+ mwifiex_queue_main_work(adapter);
/* urb for data_ep is re-submitted now;
* urb for cmd_ep will be re-submitted in callback
@@ -262,7 +262,7 @@ static void mwifiex_usb_tx_complete(struct urb *urb)
urb->status ? -1 : 0);
}
- queue_work(adapter->workqueue, &adapter->main_work);
+ mwifiex_queue_main_work(adapter);
return;
}
--
1.8.1.4
From: Zhaoyang Liu <[email protected]>
During profiling, we discovered that driver remains idle for time
when pakcet is downloaded to FW but no TX_DONE has been received
i.e. while data_sent is true.
This patch adds enhancement to TX routine where we preprocess
packets from TX queue, make them ready for TX and add them to
separate TX queue.
Signed-off-by: Zhaoyang Liu <[email protected]>
Signed-off-by: Marc Yang <[email protected]>
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/11n_aggr.c | 14 +++-
drivers/net/wireless/mwifiex/decl.h | 2 +
drivers/net/wireless/mwifiex/init.c | 5 ++
drivers/net/wireless/mwifiex/main.c | 31 ++++++--
drivers/net/wireless/mwifiex/main.h | 6 ++
drivers/net/wireless/mwifiex/txrx.c | 126 ++++++++++++++++++++++++++++++++
drivers/net/wireless/mwifiex/wmm.c | 20 ++++-
7 files changed, 189 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/11n_aggr.c b/drivers/net/wireless/mwifiex/11n_aggr.c
index 9b983b5..d8558a6 100644
--- a/drivers/net/wireless/mwifiex/11n_aggr.c
+++ b/drivers/net/wireless/mwifiex/11n_aggr.c
@@ -175,6 +175,7 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv,
struct txpd *ptx_pd = NULL;
struct timeval tv;
int headroom = adapter->iface_type == MWIFIEX_USB ? 0 : INTF_HEADER_LEN;
+ int aggr_num = 0;
skb_src = skb_peek(&pra_list->skb_head);
if (!skb_src) {
@@ -200,7 +201,8 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv,
if (tx_info_src->flags & MWIFIEX_BUF_FLAG_TDLS_PKT)
tx_info_aggr->flags |= MWIFIEX_BUF_FLAG_TDLS_PKT;
- skb_aggr->priority = skb_src->priority;
+ tx_info_aggr->flags |= MWIFIEX_BUF_FLAG_AGGR_PKT;
+ skb_aggr->priority = skb_src->priority;
do_gettimeofday(&tv);
skb_aggr->tstamp = timeval_to_ktime(tv);
@@ -211,11 +213,9 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv,
break;
skb_src = skb_dequeue(&pra_list->skb_head);
-
pra_list->total_pkt_count--;
-
atomic_dec(&priv->wmm.tx_pkts_queued);
-
+ aggr_num++;
spin_unlock_irqrestore(&priv->wmm.ra_list_spinlock,
ra_list_flags);
mwifiex_11n_form_amsdu_pkt(skb_aggr, skb_src, &pad);
@@ -251,6 +251,12 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv,
ptx_pd = (struct txpd *)skb_aggr->data;
skb_push(skb_aggr, headroom);
+ tx_info_aggr->aggr_num = aggr_num;
+ if (adapter->data_sent || adapter->tx_lock_flag) {
+ atomic_add(aggr_num, &adapter->tx_queued);
+ skb_queue_tail(&adapter->tx_data_q, skb_aggr);
+ return 0;
+ }
if (adapter->iface_type == MWIFIEX_USB) {
adapter->data_sent = true;
diff --git a/drivers/net/wireless/mwifiex/decl.h b/drivers/net/wireless/mwifiex/decl.h
index cf2fa11..f530207 100644
--- a/drivers/net/wireless/mwifiex/decl.h
+++ b/drivers/net/wireless/mwifiex/decl.h
@@ -83,6 +83,7 @@
#define MWIFIEX_BUF_FLAG_TDLS_PKT BIT(2)
#define MWIFIEX_BUF_FLAG_EAPOL_TX_STATUS BIT(3)
#define MWIFIEX_BUF_FLAG_ACTION_TX_STATUS BIT(4)
+#define MWIFIEX_BUF_FLAG_AGGR_PKT BIT(5)
#define MWIFIEX_BRIDGED_PKTS_THR_HIGH 1024
#define MWIFIEX_BRIDGED_PKTS_THR_LOW 128
@@ -179,6 +180,7 @@ struct mwifiex_txinfo {
u8 flags;
u8 bss_num;
u8 bss_type;
+ u8 aggr_num;
u32 pkt_len;
u8 ack_frame_id;
u64 cookie;
diff --git a/drivers/net/wireless/mwifiex/init.c b/drivers/net/wireless/mwifiex/init.c
index 2e1df02..2f278a6 100644
--- a/drivers/net/wireless/mwifiex/init.c
+++ b/drivers/net/wireless/mwifiex/init.c
@@ -481,6 +481,7 @@ int mwifiex_init_lock_list(struct mwifiex_adapter *adapter)
spin_lock_init(&adapter->rx_proc_lock);
skb_queue_head_init(&adapter->rx_data_q);
+ skb_queue_head_init(&adapter->tx_data_q);
for (i = 0; i < adapter->priv_num; ++i) {
INIT_LIST_HEAD(&adapter->bss_prio_tbl[i].bss_prio_head);
@@ -688,6 +689,10 @@ mwifiex_shutdown_drv(struct mwifiex_adapter *adapter)
}
}
+ atomic_set(&adapter->tx_queued, 0);
+ while ((skb = skb_dequeue(&adapter->tx_data_q)))
+ mwifiex_write_data_complete(adapter, skb, 0, 0);
+
spin_lock_irqsave(&adapter->rx_proc_lock, flags);
while ((skb = skb_dequeue(&adapter->rx_data_q))) {
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 46e1789..dcc32c1 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -259,10 +259,11 @@ process_start:
/* Need to wake up the card ? */
if ((adapter->ps_state == PS_STATE_SLEEP) &&
- (adapter->pm_wakeup_card_req &&
- !adapter->pm_wakeup_fw_try) &&
- (is_command_pending(adapter) ||
- !mwifiex_wmm_lists_empty(adapter))) {
+ (adapter->pm_wakeup_card_req &&
+ !adapter->pm_wakeup_fw_try) &&
+ (is_command_pending(adapter) ||
+ !skb_queue_empty(&adapter->tx_data_q) ||
+ !mwifiex_wmm_lists_empty(adapter))) {
adapter->pm_wakeup_fw_try = true;
mod_timer(&adapter->wakeup_timer, jiffies + (HZ*3));
adapter->if_ops.wakeup(adapter);
@@ -286,7 +287,8 @@ process_start:
if ((!adapter->scan_chan_gap_enabled &&
adapter->scan_processing) || adapter->data_sent ||
- mwifiex_wmm_lists_empty(adapter)) {
+ (skb_queue_empty(&adapter->tx_data_q) &&
+ mwifiex_wmm_lists_empty(adapter))) {
if (adapter->cmd_sent || adapter->curr_cmd ||
(!is_command_pending(adapter)))
break;
@@ -335,6 +337,19 @@ process_start:
break;
}
}
+ if ((adapter->scan_chan_gap_enabled ||
+ !adapter->scan_processing) &&
+ !skb_queue_empty(&adapter->tx_data_q) &&
+ !adapter->data_sent) {
+ mwifiex_process_tx_queue(adapter);
+ if (adapter->hs_activated) {
+ adapter->is_hs_configured = false;
+ mwifiex_hs_activated_event
+ (mwifiex_get_priv
+ (adapter, MWIFIEX_BSS_ROLE_ANY),
+ false);
+ }
+ }
if ((adapter->scan_chan_gap_enabled ||
!adapter->scan_processing) &&
@@ -350,8 +365,10 @@ process_start:
}
if (adapter->delay_null_pkt && !adapter->cmd_sent &&
- !adapter->curr_cmd && !is_command_pending(adapter) &&
- mwifiex_wmm_lists_empty(adapter)) {
+ !adapter->curr_cmd && !is_command_pending(adapter) &&
+ (mwifiex_wmm_lists_empty(adapter) &&
+ skb_queue_empty(&adapter->tx_data_q))
+ ) {
if (!mwifiex_send_null_packet
(mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_STA),
MWIFIEX_TxPD_POWER_MGMT_NULL_PACKET |
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index cc6a623..3da99e8 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -58,6 +58,8 @@ enum {
#define MWIFIEX_MAX_AP 64
+#define MWIFIEX_MAX_PKTS_TXQ 16
+
#define MWIFIEX_DEFAULT_WATCHDOG_TIMEOUT (5 * HZ)
#define MWIFIEX_TIMER_10S 10000
@@ -819,6 +821,8 @@ struct mwifiex_adapter {
/* spin lock for RX processing routine */
spinlock_t rx_proc_lock;
u32 scan_processing;
+ struct sk_buff_head tx_data_q;
+ atomic_t tx_queued;
u16 region_code;
struct mwifiex_802_11d_domain_reg domain_reg;
u16 scan_probes;
@@ -904,6 +908,8 @@ struct mwifiex_adapter {
bool auto_tdls;
};
+void mwifiex_process_tx_queue(struct mwifiex_adapter *adapter);
+
int mwifiex_init_lock_list(struct mwifiex_adapter *adapter);
void mwifiex_set_trans_start(struct net_device *dev);
diff --git a/drivers/net/wireless/mwifiex/txrx.c b/drivers/net/wireless/mwifiex/txrx.c
index ea4549f..e55a7fa 100644
--- a/drivers/net/wireless/mwifiex/txrx.c
+++ b/drivers/net/wireless/mwifiex/txrx.c
@@ -92,6 +92,12 @@ int mwifiex_process_tx(struct mwifiex_private *priv, struct sk_buff *skb,
else
head_ptr = mwifiex_process_sta_txpd(priv, skb);
+ if ((adapter->data_sent || adapter->tx_lock_flag) && head_ptr) {
+ skb_queue_tail(&adapter->tx_data_q, skb);
+ atomic_inc(&adapter->tx_queued);
+ return 0;
+ }
+
if (head_ptr) {
if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA)
local_tx_pd = (struct txpd *)(head_ptr + hroom);
@@ -142,6 +148,124 @@ int mwifiex_process_tx(struct mwifiex_private *priv, struct sk_buff *skb,
return ret;
}
+static int
+mwifiex_host_to_card(struct mwifiex_adapter *adapter, struct sk_buff *skb,
+ struct mwifiex_tx_param *tx_param)
+{
+ struct txpd *local_tx_pd = NULL;
+ u8 *head_ptr = skb->data;
+ int ret = 0;
+ struct mwifiex_private *priv;
+ struct mwifiex_txinfo *tx_info;
+
+ tx_info = MWIFIEX_SKB_TXCB(skb);
+
+ priv = mwifiex_get_priv_by_id(adapter, tx_info->bss_num,
+ tx_info->bss_type);
+ if (!priv) {
+ dev_err(adapter->dev, "data: priv not found. Drop TX packet\n");
+ adapter->dbg.num_tx_host_to_card_failure++;
+ mwifiex_write_data_complete(adapter, skb, 0, 0);
+ return ret;
+ }
+ if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA) {
+ if (adapter->iface_type == MWIFIEX_USB)
+ local_tx_pd = (struct txpd *)head_ptr;
+ else
+ local_tx_pd = (struct txpd *) (head_ptr +
+ INTF_HEADER_LEN);
+ }
+
+ if (adapter->iface_type == MWIFIEX_USB) {
+ adapter->data_sent = true;
+ ret = adapter->if_ops.host_to_card(adapter,
+ MWIFIEX_USB_EP_DATA,
+ skb, NULL);
+ } else {
+ ret = adapter->if_ops.host_to_card(adapter,
+ MWIFIEX_TYPE_DATA,
+ skb, tx_param);
+ }
+ switch (ret) {
+ case -ENOSR:
+ dev_err(adapter->dev, "data: -ENOSR is returned\n");
+ break;
+ case -EBUSY:
+ if ((GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA) &&
+ (adapter->pps_uapsd_mode && adapter->tx_lock_flag)) {
+ priv->adapter->tx_lock_flag = false;
+ if (local_tx_pd)
+ local_tx_pd->flags = 0;
+ }
+ skb_queue_head(&adapter->tx_data_q, skb);
+
+ if (tx_info->flags & MWIFIEX_BUF_FLAG_AGGR_PKT)
+ atomic_add(tx_info->aggr_num, &adapter->tx_queued);
+ else
+ atomic_inc(&adapter->tx_queued);
+
+ dev_dbg(adapter->dev, "data: -EBUSY is returned\n");
+ break;
+ case -1:
+ if (adapter->iface_type != MWIFIEX_PCIE)
+ adapter->data_sent = false;
+ dev_err(adapter->dev, "mwifiex_write_data_async failed: 0x%X\n",
+ ret);
+ adapter->dbg.num_tx_host_to_card_failure++;
+ mwifiex_write_data_complete(adapter, skb, 0, ret);
+ break;
+ case -EINPROGRESS:
+ if (adapter->iface_type != MWIFIEX_PCIE)
+ adapter->data_sent = false;
+ break;
+ case 0:
+ mwifiex_write_data_complete(adapter, skb, 0, ret);
+ break;
+ default:
+ break;
+ }
+ return ret;
+}
+
+static int
+mwifiex_dequeue_tx_queue(struct mwifiex_adapter *adapter)
+{
+ struct sk_buff *skb, *skb_next;
+ struct mwifiex_txinfo *tx_info;
+ struct mwifiex_tx_param tx_param;
+
+ skb = skb_dequeue(&adapter->tx_data_q);
+
+ if (!skb)
+ return -1;
+
+ tx_info = MWIFIEX_SKB_TXCB(skb);
+ if (tx_info->flags & MWIFIEX_BUF_FLAG_AGGR_PKT)
+ atomic_sub(tx_info->aggr_num, &adapter->tx_queued);
+ else
+ atomic_dec(&adapter->tx_queued);
+
+ if (!skb_queue_empty(&adapter->tx_data_q))
+ skb_next = skb_peek(&adapter->tx_data_q);
+ else
+ skb_next = NULL;
+
+ tx_param.next_pkt_len = ((skb_next) ? skb_next->len : 0);
+
+ return mwifiex_host_to_card(adapter, skb, &tx_param);
+}
+
+void
+mwifiex_process_tx_queue(struct mwifiex_adapter *adapter)
+{
+ do {
+ if (adapter->data_sent || adapter->tx_lock_flag)
+ break;
+ if (mwifiex_dequeue_tx_queue(adapter))
+ break;
+ } while (!skb_queue_empty(&adapter->tx_data_q));
+}
+
/*
* Packet send completion callback handler.
*
@@ -181,6 +305,8 @@ int mwifiex_write_data_complete(struct mwifiex_adapter *adapter,
if (tx_info->flags & MWIFIEX_BUF_FLAG_BRIDGED_PKT)
atomic_dec_return(&adapter->pending_bridged_pkts);
+ if (tx_info->flags & MWIFIEX_BUF_FLAG_AGGR_PKT)
+ goto done;
if (aggr)
/* For skb_aggr, do not wake up tx queue */
diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c
index ce747f7..c53c769 100644
--- a/drivers/net/wireless/mwifiex/wmm.c
+++ b/drivers/net/wireless/mwifiex/wmm.c
@@ -1174,6 +1174,14 @@ mwifiex_send_processed_packet(struct mwifiex_private *priv,
skb = skb_dequeue(&ptr->skb_head);
+ if (adapter->data_sent || adapter->tx_lock_flag) {
+ spin_unlock_irqrestore(&priv->wmm.ra_list_spinlock,
+ ra_list_flags);
+ skb_queue_tail(&adapter->tx_data_q, skb);
+ atomic_inc(&adapter->tx_queued);
+ return;
+ }
+
if (!skb_queue_empty(&ptr->skb_head))
skb_next = skb_peek(&ptr->skb_head);
else
@@ -1324,11 +1332,15 @@ void
mwifiex_wmm_process_tx(struct mwifiex_adapter *adapter)
{
do {
- /* Check if busy */
- if (adapter->data_sent || adapter->tx_lock_flag)
- break;
-
if (mwifiex_dequeue_tx_packet(adapter))
break;
+ if (adapter->iface_type != MWIFIEX_SDIO) {
+ if (adapter->data_sent || adapter->tx_lock_flag)
+ break;
+ } else {
+ if (atomic_read(&adapter->tx_queued) >=
+ MWIFIEX_MAX_PKTS_TXQ)
+ break;
+ }
} while (!mwifiex_wmm_lists_empty(adapter));
}
--
1.8.1.4
Hi Kalle,
> -----Original Message-----
> From: Kalle Valo [mailto:[email protected]]
> Sent: Tuesday, March 03, 2015 6:08 PM
> To: Avinash Patil
> Cc: [email protected]; Amitkumar Karwar; Cathy Luo; Zhaoyang Liu;
> Shengzhen Li; Marc Yang
> Subject: Re: [PATCH 1/4] mwifiex: avoid queue_work while work is ongoing
>
> Avinash Patil <[email protected]> writes:
>
> > From: Marc Yang <[email protected]>
> >
> > Current code does not check whether main_work_queue or rx_work_queue
> > is running when preparing to do queue_work, this code fix add check
> > before calling queue_work, reducing unnecessary queue_work switch.
> >
> > This change instead sets more_task flag to ensure we run main_process
> > superloop once again.
> >
> > Signed-off-by: Marc Yang <[email protected]>
> > Signed-off-by: Zhaoyang Liu <[email protected]>
> > Signed-off-by: Shengzhen Li <[email protected]>
> > Signed-off-by: Cathy Luo <[email protected]>
> > Signed-off-by: Amitkumar Karwar <[email protected]>
> > Signed-off-by: Avinash Patil <[email protected]>
>
> Really, it took six persons to write this patch? Or do you just dump the same
> names to each patch?
This should have been "Reviewed-by". Apologies.
> > +void mwifiex_queue_main_work(struct mwifiex_adapter *adapter) {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&adapter->main_proc_lock, flags);
> > + if (adapter->mwifiex_processing) {
> > + adapter->more_task_flag = true;
> > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> > + } else {
> > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> > + queue_work(adapter->workqueue, &adapter->main_work);
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(mwifiex_queue_main_work);
> > +
> > +static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter) {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&adapter->rx_proc_lock, flags);
> > + if (adapter->rx_processing) {
> > + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> > + } else {
> > + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> > + queue_work(adapter->rx_workqueue, &adapter->rx_work);
> > + }
> > +}
>
> I can apply this patch, but to me this looks like a horrible hack.
We are here trying to avoid requeueing another work when work item is being executed. We set more_task flag to true if work item is being executed and mwifiex_main_process would execute superloop once again if more_task it set. work_pending() cannot be of much help here since we want to avoid queing only when work item is being executed. Could you please let us know if you think of any better way to handle this?
> --
> Kalle Valo
-Avinash
Avinash Patil <[email protected]> writes:
> From: Zhaoyang Liu <[email protected]>
>
> This patch does away with spinlock in
> mwifiex_wmm_get_highest_priolist_ptr in order to improve TP.
>
> Signed-off-by: Zhaoyang Liu <[email protected]>
> Signed-off-by: Shengzhen Li <[email protected]>
> Signed-off-by: Cathy Luo <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> Signed-off-by: Avinash Patil <[email protected]>
[...]
> @@ -727,6 +730,25 @@ static int mwifiex_deinit_priv_params(struct mwifiex_private *priv)
> }
>
> mwifiex_deauthenticate(priv, NULL);
> +
> + spin_lock_irqsave(&adapter->main_proc_lock, flags);
> + adapter->main_locked = true;
> + if (adapter->mwifiex_processing) {
> + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> + flush_workqueue(adapter->workqueue);
> + } else {
> + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> + }
> +
> + spin_lock_irqsave(&adapter->rx_proc_lock, flags);
> + adapter->rx_locked = true;
> + if (adapter->rx_processing) {
> + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> + flush_workqueue(adapter->rx_workqueue);
> + } else {
> + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> + }
And here the horrible hack just grows to other functions.
--
Kalle Valo
Hi Kalle,
Please hold on.
I am planning to divide patch3 into 2 patches - I will send updated v2.
Thanks,
Avinash
________________________________________
From: Kalle Valo [[email protected]]
Sent: Thursday, March 05, 2015 5:03 PM
To: Avinash Patil
Cc: [email protected]; Amitkumar Karwar; Cathy Luo; Zhaoyang Liu; Shengzhen Li
Subject: Re: [PATCH 1/4] mwifiex: avoid queue_work while work is ongoing
Avinash Patil <[email protected]> writes:
>> > From: Marc Yang <[email protected]>
>> >
>> > Current code does not check whether main_work_queue or rx_work_queue
>> > is running when preparing to do queue_work, this code fix add check
>> > before calling queue_work, reducing unnecessary queue_work switch.
>> >
>> > This change instead sets more_task flag to ensure we run main_process
>> > superloop once again.
>> >
>> > Signed-off-by: Marc Yang <[email protected]>
>> > Signed-off-by: Zhaoyang Liu <[email protected]>
>> > Signed-off-by: Shengzhen Li <[email protected]>
>> > Signed-off-by: Cathy Luo <[email protected]>
>> > Signed-off-by: Amitkumar Karwar <[email protected]>
>> > Signed-off-by: Avinash Patil <[email protected]>
>>
>> Really, it took six persons to write this patch? Or do you just dump the same
>> names to each patch?
>
> This should have been "Reviewed-by". Apologies.
Ok, Reviewed-by would make more sense here. Please use that in the
future (no need to change anything for these patches).
>> > +void mwifiex_queue_main_work(struct mwifiex_adapter *adapter) {
>> > + unsigned long flags;
>> > +
>> > + spin_lock_irqsave(&adapter->main_proc_lock, flags);
>> > + if (adapter->mwifiex_processing) {
>> > + adapter->more_task_flag = true;
>> > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>> > + } else {
>> > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>> > + queue_work(adapter->workqueue, &adapter->main_work);
>> > + }
>> > +}
>> > +EXPORT_SYMBOL_GPL(mwifiex_queue_main_work);
>> > +
>> > +static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter) {
>> > + unsigned long flags;
>> > +
>> > + spin_lock_irqsave(&adapter->rx_proc_lock, flags);
>> > + if (adapter->rx_processing) {
>> > + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
>> > + } else {
>> > + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
>> > + queue_work(adapter->rx_workqueue, &adapter->rx_work);
>> > + }
>> > +}
>>
>> I can apply this patch, but to me this looks like a horrible hack.
>
> We are here trying to avoid requeueing another work when work item is
> being executed. We set more_task flag to true if work item is being
> executed and mwifiex_main_process would execute superloop once again
> if more_task it set. work_pending() cannot be of much help here since
> we want to avoid queing only when work item is being executed. Could
> you please let us know if you think of any better way to handle this?
It would be best to modify workqueue code to support this instead of
doing this in the driver. But this was just an observation I made while
reviewing your patches, I'll apply these anyway.
--
Kalle Valo
Avinash Patil <[email protected]> writes:
> Please hold on.
> I am planning to divide patch3 into 2 patches - I will send updated v2.
Ok, I'll wait for v2.
--
Kalle Valo
Avinash Patil <[email protected]> writes:
>> > From: Marc Yang <[email protected]>
>> >
>> > Current code does not check whether main_work_queue or rx_work_queue
>> > is running when preparing to do queue_work, this code fix add check
>> > before calling queue_work, reducing unnecessary queue_work switch.
>> >
>> > This change instead sets more_task flag to ensure we run main_process
>> > superloop once again.
>> >
>> > Signed-off-by: Marc Yang <[email protected]>
>> > Signed-off-by: Zhaoyang Liu <[email protected]>
>> > Signed-off-by: Shengzhen Li <[email protected]>
>> > Signed-off-by: Cathy Luo <[email protected]>
>> > Signed-off-by: Amitkumar Karwar <[email protected]>
>> > Signed-off-by: Avinash Patil <[email protected]>
>>
>> Really, it took six persons to write this patch? Or do you just dump the same
>> names to each patch?
>
> This should have been "Reviewed-by". Apologies.
Ok, Reviewed-by would make more sense here. Please use that in the
future (no need to change anything for these patches).
>> > +void mwifiex_queue_main_work(struct mwifiex_adapter *adapter) {
>> > + unsigned long flags;
>> > +
>> > + spin_lock_irqsave(&adapter->main_proc_lock, flags);
>> > + if (adapter->mwifiex_processing) {
>> > + adapter->more_task_flag = true;
>> > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>> > + } else {
>> > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>> > + queue_work(adapter->workqueue, &adapter->main_work);
>> > + }
>> > +}
>> > +EXPORT_SYMBOL_GPL(mwifiex_queue_main_work);
>> > +
>> > +static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter) {
>> > + unsigned long flags;
>> > +
>> > + spin_lock_irqsave(&adapter->rx_proc_lock, flags);
>> > + if (adapter->rx_processing) {
>> > + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
>> > + } else {
>> > + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
>> > + queue_work(adapter->rx_workqueue, &adapter->rx_work);
>> > + }
>> > +}
>>
>> I can apply this patch, but to me this looks like a horrible hack.
>
> We are here trying to avoid requeueing another work when work item is
> being executed. We set more_task flag to true if work item is being
> executed and mwifiex_main_process would execute superloop once again
> if more_task it set. work_pending() cannot be of much help here since
> we want to avoid queing only when work item is being executed. Could
> you please let us know if you think of any better way to handle this?
It would be best to modify workqueue code to support this instead of
doing this in the driver. But this was just an observation I made while
reviewing your patches, I'll apply these anyway.
--
Kalle Valo
Avinash Patil <[email protected]> writes:
> From: Marc Yang <[email protected]>
>
> Current code does not check whether main_work_queue or
> rx_work_queue is running when preparing to do queue_work,
> this code fix add check before calling queue_work, reducing
> unnecessary queue_work switch.
>
> This change instead sets more_task flag to ensure we run main_process
> superloop once again.
>
> Signed-off-by: Marc Yang <[email protected]>
> Signed-off-by: Zhaoyang Liu <[email protected]>
> Signed-off-by: Shengzhen Li <[email protected]>
> Signed-off-by: Cathy Luo <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> Signed-off-by: Avinash Patil <[email protected]>
Really, it took six persons to write this patch? Or do you just dump the
same names to each patch?
> +void mwifiex_queue_main_work(struct mwifiex_adapter *adapter)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adapter->main_proc_lock, flags);
> + if (adapter->mwifiex_processing) {
> + adapter->more_task_flag = true;
> + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> + } else {
> + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> + queue_work(adapter->workqueue, &adapter->main_work);
> + }
> +}
> +EXPORT_SYMBOL_GPL(mwifiex_queue_main_work);
> +
> +static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adapter->rx_proc_lock, flags);
> + if (adapter->rx_processing) {
> + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> + } else {
> + spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> + queue_work(adapter->rx_workqueue, &adapter->rx_work);
> + }
> +}
I can apply this patch, but to me this looks like a horrible hack.
--
Kalle Valo