diff mbox series

[v2] ath11k: fix htt stats module not handle multiple skbs

Message ID 1589221074-28778-1-git-send-email-pradeepc@codeaurora.org (mailing list archive)
State Accepted
Commit 012f81456818dfb49d2939373b163945da3a4032
Delegated to: Kalle Valo
Headers show
Series [v2] ath11k: fix htt stats module not handle multiple skbs | expand

Commit Message

Pradeep Kumar Chitrapu May 11, 2020, 6:17 p.m. UTC 
HTT EXT stats comes in stream of TLVs spanning over multiple
messages. Currently completion is being sent for each message
which is creating a race where stats_req is being accessed
for filling in second message after the memory is already
freed in release operation. Fix this by issuing completion
once all the messages are received and processed. Driver
knows this info from DONE bit set in htt msg.

Also fix locking required for htt stats.

Co-developed-by: Miles Hu <milehu@codeaurora.org>
Signed-off-by: Miles Hu <milehu@codeaurora.org>
Signed-off-by: Pradeep Kumar Chitrapu <pradeepc@codeaurora.org>
---
v2:
 - fix kbot reported issue of missing unlock

 drivers/net/wireless/ath/ath11k/debug_htt_stats.c | 48 ++++++++++++++++++-----
 drivers/net/wireless/ath/ath11k/dp.h              |  1 +
 2 files changed, 39 insertions(+), 10 deletions(-)

Comments

Kalle Valo May 13, 2020, 4:02 p.m. UTC | #1 
Pradeep Kumar Chitrapu <pradeepc@codeaurora.org> wrote:

> HTT EXT stats comes in stream of TLVs spanning over multiple
> messages. Currently completion is being sent for each message
> which is creating a race where stats_req is being accessed
> for filling in second message after the memory is already
> freed in release operation. Fix this by issuing completion
> once all the messages are received and processed. Driver
> knows this info from DONE bit set in htt msg.
> 
> Also fix locking required for htt stats.
> 
> Co-developed-by: Miles Hu <milehu@codeaurora.org>
> Signed-off-by: Miles Hu <milehu@codeaurora.org>
> Signed-off-by: Pradeep Kumar Chitrapu <pradeepc@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

012f81456818 ath11k: fix htt stats module not handle multiple skbs
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/debug_htt_stats.c b/drivers/net/wireless/ath/ath11k/debug_htt_stats.c
index 5db0c27de475..6b532dc99c98 100644
--- a/drivers/net/wireless/ath/ath11k/debug_htt_stats.c
+++ b/drivers/net/wireless/ath/ath11k/debug_htt_stats.c
@@ -4306,6 +4306,7 @@  void ath11k_dbg_htt_ext_stats_handler(struct ath11k_base *ab,
 	u32 len;
 	u64 cookie;
 	int ret;
+	bool send_completion = false;
 	u8 pdev_id;
 
 	msg = (struct ath11k_htt_extd_stats_msg *)skb->data;
@@ -4330,11 +4331,11 @@  void ath11k_dbg_htt_ext_stats_handler(struct ath11k_base *ab,
 		return;
 
 	spin_lock_bh(&ar->debug.htt_stats.lock);
-	if (stats_req->done) {
-		spin_unlock_bh(&ar->debug.htt_stats.lock);
-		return;
-	}
-	stats_req->done = true;
+
+	stats_req->done = FIELD_GET(HTT_T2H_EXT_STATS_INFO1_DONE, msg->info1);
+	if (stats_req->done)
+		send_completion = true;
+
 	spin_unlock_bh(&ar->debug.htt_stats.lock);
 
 	len = FIELD_GET(HTT_T2H_EXT_STATS_INFO1_LENGTH, msg->info1);
@@ -4344,7 +4345,8 @@  void ath11k_dbg_htt_ext_stats_handler(struct ath11k_base *ab,
 	if (ret)
 		ath11k_warn(ab, "Failed to parse tlv %d\n", ret);
 
-	complete(&stats_req->cmpln);
+	if (send_completion)
+		complete(&stats_req->cmpln);
 }
 
 static ssize_t ath11k_read_htt_stats_type(struct file *file,
@@ -4497,28 +4499,54 @@  static int ath11k_open_htt_stats(struct inode *inode, struct file *file)
 	if (type == ATH11K_DBG_HTT_EXT_STATS_RESET)
 		return -EPERM;
 
+	mutex_lock(&ar->conf_mutex);
+
+	if (ar->state != ATH11K_STATE_ON) {
+		ret = -ENETDOWN;
+		goto err_unlock;
+	}
+
+	if (ar->debug.htt_stats.stats_req) {
+		ret = -EAGAIN;
+		goto err_unlock;
+	}
+
 	stats_req = vzalloc(sizeof(*stats_req) + ATH11K_HTT_STATS_BUF_SIZE);
-	if (!stats_req)
-		return -ENOMEM;
+	if (!stats_req) {
+		ret = -ENOMEM;
+		goto err_unlock;
+	}
 
-	mutex_lock(&ar->conf_mutex);
 	ar->debug.htt_stats.stats_req = stats_req;
 	stats_req->type = type;
+
 	ret = ath11k_dbg_htt_stats_req(ar);
-	mutex_unlock(&ar->conf_mutex);
 	if (ret < 0)
 		goto out;
 
 	file->private_data = stats_req;
+
+	mutex_unlock(&ar->conf_mutex);
+
 	return 0;
 out:
 	vfree(stats_req);
+	ar->debug.htt_stats.stats_req = NULL;
+err_unlock:
+	mutex_unlock(&ar->conf_mutex);
+
 	return ret;
 }
 
 static int ath11k_release_htt_stats(struct inode *inode, struct file *file)
 {
+	struct ath11k *ar = inode->i_private;
+
+	mutex_lock(&ar->conf_mutex);
 	vfree(file->private_data);
+	ar->debug.htt_stats.stats_req = NULL;
+	mutex_unlock(&ar->conf_mutex);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/ath/ath11k/dp.h b/drivers/net/wireless/ath/ath11k/dp.h
index 222de10e4b93..058a5c1d86ff 100644
--- a/drivers/net/wireless/ath/ath11k/dp.h
+++ b/drivers/net/wireless/ath/ath11k/dp.h
@@ -1517,6 +1517,7 @@  struct htt_ext_stats_cfg_params {
  *       4 bytes.
  */
 
+#define HTT_T2H_EXT_STATS_INFO1_DONE	BIT(11)
 #define HTT_T2H_EXT_STATS_INFO1_LENGTH   GENMASK(31, 16)
 
 struct ath11k_htt_extd_stats_msg {