From: Raja Mani <[email protected]>
ath6kl_tm_rx_report() func takes ar->sem and sends tcmd to the chip
and then waits for wake_up event from ath6kl_tm_rx_report_event() with timeout.
In the current case, When tcmd report is reached to the host,
ath6kl_tm_rx_report_event() func tries to take the same semaphore (ar->sem)
which is already taken in ath6kl_tm_rx_report().
Due to this, ath6kl_tm_rx_report_event() func always fails to update
tcmd report in the local buffer and sends wake_up event to ath6kl_tm_rx_report().
So, the timeout will happen in ath6kl_tm_rx_report() always and
then it will release ar->sem. Now ath6kl_tm_rx_report_event() will
get a chance to update tcmd report in the local buffer.
There is no need of taking ar->sem in ath6kl_tm_rx_report_event(), we can go ahead
and update the local buffer and send wake_up event to ath6kl_tm_rx_report().
In this way, we will get tcmd report (in the user space) in the first time itself.
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/testmode.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/testmode.c b/drivers/net/wireless/ath/ath6kl/testmode.c
index 381eb66..971ea61 100644
--- a/drivers/net/wireless/ath/ath6kl/testmode.c
+++ b/drivers/net/wireless/ath/ath6kl/testmode.c
@@ -43,16 +43,11 @@ static const struct nla_policy ath6kl_tm_policy[ATH6KL_TM_ATTR_MAX + 1] = {
void ath6kl_tm_rx_report_event(struct ath6kl *ar, void *buf, size_t buf_len)
{
- if (down_interruptible(&ar->sem))
- return;
-
kfree(ar->tm.rx_report);
ar->tm.rx_report = kmemdup(buf, buf_len, GFP_KERNEL);
ar->tm.rx_report_len = buf_len;
- up(&ar->sem);
-
wake_up(&ar->event_wq);
}
--
1.7.0.4
On 09/06/2011 02:05 PM, [email protected] wrote:
> From: Raja Mani <[email protected]>
>
> ath6kl_tm_rx_report() func takes ar->sem and sends tcmd to the chip
> and then waits for wake_up event from ath6kl_tm_rx_report_event() with timeout.
>
> In the current case, When tcmd report is reached to the host,
> ath6kl_tm_rx_report_event() func tries to take the same semaphore (ar->sem)
> which is already taken in ath6kl_tm_rx_report().
>
> Due to this, ath6kl_tm_rx_report_event() func always fails to update
> tcmd report in the local buffer and sends wake_up event to ath6kl_tm_rx_report().
> So, the timeout will happen in ath6kl_tm_rx_report() always and
> then it will release ar->sem. Now ath6kl_tm_rx_report_event() will
> get a chance to update tcmd report in the local buffer.
This makes sense. I remember seeing something similar myself.
> There is no need of taking ar->sem in ath6kl_tm_rx_report_event(), we can go ahead
> and update the local buffer and send wake_up event to ath6kl_tm_rx_report().
> In this way, we will get tcmd report (in the user space) in the first time itself.
To me that looks racy. What will then prevent concurrent access to
ar->tm.rx_report?
Wouldn't it be better to release semaphore beforing calling
wait_event_interruptible_timeout() and then take it again after the
event has happened?
Kalle