wlan: Use spinlock to synchronize SME request/response
Many of the APIs supported by HDD require a call to SME to perform an
action or to retrieve some data. In most cases SME performs the
operation asynchronously, and will execute a provided callback
function when the request has completed. In order to synchronize this
the HDD API allocates a context which is then passed to SME, and which
is then, in turn, passed back to the callback function when the
operation completes. The callback function then sets a completion
variable inside the context which the HDD API is waiting on. In an
ideal world the HDD API would wait forever (or at least for a long
time) for the response to be received and for the completion variable
to be set. However in most cases these HDD APIs are being invoked in
the context of a userspace thread which has invoked either a cfg80211
API or a wireless extensions ioctl and which has taken the kernel
rtnl_lock. Since this lock is used to synchronize many of the kernel
tasks, we do not want to hold it for a long time. In addition we do
not want to block userspace threads (such as the wpa supplicant's main
thread) for an extended time. Therefore we only block for a short
time waiting for the response before we timeout. This means that it
is possible for the HDD API to timeout, and for the callback to be
invoked afterwards. In order for the callback function to determine
if the HDD API is still waiting, a magic value is also stored in the
shared context. Only if the context has a valid magic will the
callback routine do any work.
The original implementation of this logic included a well-documented
race condition where the HDD API timeout could coincide with the
callback being invoked. In such a scenario the HDD API could be
invalidating the context immediately after the callback function had
validated it and was dereferencing it. In order to mitigate the
effects of such a race condition, the HDD API would always msleep()
for a short amount of time when a timeout occurred so that if the
callback was executing concurrently, the callback could (hopefully)
complete its work before the HDD API exited.
Due to an customer-observed issue this logic was further scrutinized
and it was suggested that a spinlock could be used to properly
serialize the activities of the HDD API and the callback function when
they are running in parallel. So introduce a spinlock that is used so
that if any HDD API timeout coincides with its callback, the
operations of the two threads will be serialized.
Change-Id: If0d84e535f6cf0818410e4210f5bfe8294919526
CRs-fixed: 592261
diff --git a/CORE/HDD/inc/wlan_hdd_main.h b/CORE/HDD/inc/wlan_hdd_main.h
index 60bc5dd..79149fa 100644
--- a/CORE/HDD/inc/wlan_hdd_main.h
+++ b/CORE/HDD/inc/wlan_hdd_main.h
@@ -227,6 +227,37 @@
#endif
+/*
+ * Generic asynchronous request/response support
+ *
+ * Many of the APIs supported by HDD require a call to SME to
+ * perform an action or to retrieve some data. In most cases SME
+ * performs the operation asynchronously, and will execute a provided
+ * callback function when the request has completed. In order to
+ * synchronize this the HDD API allocates a context which is then
+ * passed to SME, and which is then, in turn, passed back to the
+ * callback function when the operation completes. The callback
+ * function then sets a completion variable inside the context which
+ * the HDD API is waiting on. In an ideal world the HDD API would
+ * wait forever (or at least for a long time) for the response to be
+ * received and for the completion variable to be set. However in
+ * most cases these HDD APIs are being invoked in the context of a
+ * userspace thread which has invoked either a cfg80211 API or a
+ * wireless extensions ioctl and which has taken the kernel rtnl_lock.
+ * Since this lock is used to synchronize many of the kernel tasks, we
+ * do not want to hold it for a long time. In addition we do not want
+ * to block userspace threads (such as the wpa supplicant's main
+ * thread) for an extended time. Therefore we only block for a short
+ * time waiting for the response before we timeout. This means that
+ * it is possible for the HDD API to timeout, and for the callback to
+ * be invoked afterwards. In order for the callback function to
+ * determine if the HDD API is still waiting, a magic value is also
+ * stored in the shared context. Only if the context has a valid
+ * magic will the callback routine do any work. In order to further
+ * synchronize these activities a spinlock is used so that if any HDD
+ * API timeout coincides with its callback, the operations of the two
+ * threads will be serialized.
+ */
struct statsContext
{
struct completion completion;
@@ -234,7 +265,12 @@
unsigned int magic;
};
+extern spinlock_t hdd_context_lock;
+
#define STATS_CONTEXT_MAGIC 0x53544154 //STAT
+#define RSSI_CONTEXT_MAGIC 0x52535349 //RSSI
+#define POWER_CONTEXT_MAGIC 0x504F5752 //POWR
+#define SNR_CONTEXT_MAGIC 0x534E5200 //SNR
typedef struct hdd_tx_rx_stats_s
{
diff --git a/CORE/HDD/src/wlan_hdd_hostapd.c b/CORE/HDD/src/wlan_hdd_hostapd.c
index 688ce17..af86402 100644
--- a/CORE/HDD/src/wlan_hdd_hostapd.c
+++ b/CORE/HDD/src/wlan_hdd_hostapd.c
@@ -99,7 +99,6 @@
#define IS_UP_AUTO(_ic) \
(IS_UP((_ic)->ic_dev) && (_ic)->ic_roaming == IEEE80211_ROAMING_AUTO)
#define WE_WLAN_VERSION 1
-#define STATS_CONTEXT_MAGIC 0x53544154
#define WE_GET_STA_INFO_SIZE 30
/* WEXT limition: MAX allowed buf len for any *
* IW_PRIV_TYPE_CHAR is 2Kbytes *
@@ -2813,15 +2812,29 @@
{
lrc = wait_for_completion_interruptible_timeout(&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_STATS));
- context.magic = 0;
if (lrc <= 0)
{
hddLog(VOS_TRACE_LEVEL_ERROR,
"%s: SME %s while retrieving link speed",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- msleep(50);
}
}
+
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
return VOS_STATUS_SUCCESS;
}
diff --git a/CORE/HDD/src/wlan_hdd_main.c b/CORE/HDD/src/wlan_hdd_main.c
index d3b291a..95fb6ec 100755
--- a/CORE/HDD/src/wlan_hdd_main.c
+++ b/CORE/HDD/src/wlan_hdd_main.c
@@ -161,6 +161,12 @@
#endif
/*
+ * spinlock for synchronizing asynchronous request/response
+ * (full description of use in wlan_hdd_main.h)
+ */
+DEFINE_SPINLOCK(hdd_context_lock);
+
+/*
* The rate at which the driver sends RESTART event to supplicant
* once the function 'vos_wlanRestart()' is called
*
@@ -3842,25 +3848,30 @@
return;
}
- /* there is a race condition that exists between this callback function
- and the caller since the caller could time out either before or
- while this code is executing. we'll assume the timeout hasn't
- occurred, but we'll verify that right before we save our work */
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
pStatsContext = pContext;
pAdapter = pStatsContext->pAdapter;
if ((NULL == pAdapter) || (STATS_CONTEXT_MAGIC != pStatsContext->magic))
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
"%s: Invalid context, pAdapter [%p] magic [%08x]",
__func__, pAdapter, pStatsContext->magic);
return;
}
- /* the race is on. caller could have timed out immediately after
- we verified the magic, but if so, caller will wait a short time
- for us to copy over the tsm stats */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pStatsContext->magic = 0;
+
+ /* copy over the tsm stats */
pAdapter->tsmStats.UplinkPktQueueDly = tsmMetrics.UplinkPktQueueDly;
vos_mem_copy(pAdapter->tsmStats.UplinkPktQueueDlyHist,
tsmMetrics.UplinkPktQueueDlyHist,
@@ -3872,8 +3883,11 @@
pAdapter->tsmStats.RoamingCount = tsmMetrics.RoamingCount;
pAdapter->tsmStats.RoamingDly = tsmMetrics.RoamingDly;
- /* and notify the caller */
+ /* notify the caller */
complete(&pStatsContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
@@ -3883,6 +3897,7 @@
{
hdd_station_ctx_t *pHddStaCtx = NULL;
eHalStatus hstatus;
+ VOS_STATUS vstatus = VOS_STATUS_SUCCESS;
long lrc;
struct statsContext context;
hdd_context_t *pHddCtx = NULL;
@@ -3909,45 +3924,53 @@
if (eHAL_STATUS_SUCCESS != hstatus)
{
- hddLog(VOS_TRACE_LEVEL_ERROR, "%s: Unable to retrieve statistics", __func__);
- return hstatus;
+ hddLog(VOS_TRACE_LEVEL_ERROR, "%s: Unable to retrieve statistics",
+ __func__);
+ vstatus = VOS_STATUS_E_FAULT;
}
else
{
/* request was sent -- wait for the response */
lrc = wait_for_completion_interruptible_timeout(&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_STATS));
- /* either we have a response or we timed out
- either way, first invalidate our magic */
- context.magic = 0;
if (lrc <= 0)
{
hddLog(VOS_TRACE_LEVEL_ERROR,
"%s: SME %s while retrieving statistics",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
- return (VOS_STATUS_E_TIMEOUT);
+ vstatus = VOS_STATUS_E_TIMEOUT;
}
}
- pTsmMetrics->UplinkPktQueueDly = pAdapter->tsmStats.UplinkPktQueueDly;
- vos_mem_copy(pTsmMetrics->UplinkPktQueueDlyHist,
- pAdapter->tsmStats.UplinkPktQueueDlyHist,
- sizeof(pAdapter->tsmStats.UplinkPktQueueDlyHist)/
- sizeof(pAdapter->tsmStats.UplinkPktQueueDlyHist[0]));
- pTsmMetrics->UplinkPktTxDly = pAdapter->tsmStats.UplinkPktTxDly;
- pTsmMetrics->UplinkPktLoss = pAdapter->tsmStats.UplinkPktLoss;
- pTsmMetrics->UplinkPktCount = pAdapter->tsmStats.UplinkPktCount;
- pTsmMetrics->RoamingCount = pAdapter->tsmStats.RoamingCount;
- pTsmMetrics->RoamingDly = pAdapter->tsmStats.RoamingDly;
- return VOS_STATUS_SUCCESS;
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
+ if (VOS_STATUS_SUCCESS == vstatus)
+ {
+ pTsmMetrics->UplinkPktQueueDly = pAdapter->tsmStats.UplinkPktQueueDly;
+ vos_mem_copy(pTsmMetrics->UplinkPktQueueDlyHist,
+ pAdapter->tsmStats.UplinkPktQueueDlyHist,
+ sizeof(pAdapter->tsmStats.UplinkPktQueueDlyHist)/
+ sizeof(pAdapter->tsmStats.UplinkPktQueueDlyHist[0]));
+ pTsmMetrics->UplinkPktTxDly = pAdapter->tsmStats.UplinkPktTxDly;
+ pTsmMetrics->UplinkPktLoss = pAdapter->tsmStats.UplinkPktLoss;
+ pTsmMetrics->UplinkPktCount = pAdapter->tsmStats.UplinkPktCount;
+ pTsmMetrics->RoamingCount = pAdapter->tsmStats.RoamingCount;
+ pTsmMetrics->RoamingDly = pAdapter->tsmStats.RoamingDly;
+ }
+ return vstatus;
}
#endif /*FEATURE_WLAN_CCX && FEATURE_WLAN_CCX_UPLOAD */
@@ -6865,13 +6888,6 @@
vos_mem_free(scanReq.ChannelInfo.ChannelList);
}
-struct fullPowerContext
-{
- struct completion completion;
- unsigned int magic;
-};
-#define POWER_CONTEXT_MAGIC 0x504F5752 //POWR
-
/**---------------------------------------------------------------------------
\brief hdd_full_power_callback() - HDD full power callback function
@@ -6887,7 +6903,7 @@
--------------------------------------------------------------------------*/
static void hdd_full_power_callback(void *callbackContext, eHalStatus status)
{
- struct fullPowerContext *pContext = callbackContext;
+ struct statsContext *pContext = callbackContext;
hddLog(VOS_TRACE_LEVEL_INFO,
"%s: context = %p, status = %d", __func__, pContext, status);
@@ -6900,24 +6916,32 @@
return;
}
- /* there is a race condition that exists between this callback function
- and the caller since the caller could time out either before or
- while this code is executing. we'll assume the timeout hasn't
- occurred, but we'll verify that right before we save our work */
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
if (POWER_CONTEXT_MAGIC != pContext->magic)
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
"%s: Invalid context, magic [%08x]",
__func__, pContext->magic);
return;
}
- /* the race is on. caller could have timed out immediately after
- we verified the magic, but if so, caller will wait a short time
- for us to notify the caller, so the context will stay valid */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pContext->magic = 0;
+
+ /* notify the caller */
complete(&pContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
/**---------------------------------------------------------------------------
@@ -6938,7 +6962,7 @@
VOS_STATUS vosStatus;
struct wiphy *wiphy = pHddCtx->wiphy;
hdd_adapter_t* pAdapter;
- struct fullPowerContext powerContext;
+ struct statsContext powerContext;
long lrc;
ENTER();
@@ -7059,21 +7083,10 @@
lrc = wait_for_completion_interruptible_timeout(
&powerContext.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_POWER));
- /* either we have a response or we timed out
- either way, first invalidate our magic */
- powerContext.magic = 0;
if (lrc <= 0)
{
hddLog(VOS_TRACE_LEVEL_ERROR, "%s: %s while requesting full power",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
}
}
else
@@ -7081,11 +7094,25 @@
hddLog(VOS_TRACE_LEVEL_ERROR,
"%s: Request for Full Power failed, status %d",
__func__, halStatus);
- VOS_ASSERT(0);
/* continue -- need to clean up as much as possible */
}
}
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ powerContext.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
hdd_debugfs_exit(pHddCtx);
// Unregister the Net Device Notifier
diff --git a/CORE/HDD/src/wlan_hdd_wext.c b/CORE/HDD/src/wlan_hdd_wext.c
index 6461189..5544128 100644
--- a/CORE/HDD/src/wlan_hdd_wext.c
+++ b/CORE/HDD/src/wlan_hdd_wext.c
@@ -131,11 +131,6 @@
static int ioctl_debug;
module_param(ioctl_debug, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-#define STATS_CONTEXT_MAGIC 0x53544154 //STAT
-#define RSSI_CONTEXT_MAGIC 0x52535349 //RSSI
-#define POWER_CONTEXT_MAGIC 0x504F5752 //POWR
-#define SNR_CONTEXT_MAGIC 0x534E5200 //SNR
-
/* To Validate Channel against the Frequency and Vice-Versa */
static const hdd_freq_chan_map_t freq_chan_map[] = { {2412, 1}, {2417, 2},
{2422, 3}, {2427, 4}, {2432, 5}, {2437, 6}, {2442, 7}, {2447, 8},
@@ -580,16 +575,19 @@
return;
}
- /* there is a race condition that exists between this callback function
- and the caller since the caller could time out either before or
- while this code is executing. we'll assume the timeout hasn't
- occurred, but we'll verify that right before we save our work */
-
pStatsContext = pContext;
pAdapter = pStatsContext->pAdapter;
+
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
+
if ((NULL == pAdapter) || (RSSI_CONTEXT_MAGIC != pStatsContext->magic))
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
"%s: Invalid context, pAdapter [%p] magic [%08x]",
__func__, pAdapter, pStatsContext->magic);
@@ -601,13 +599,19 @@
return;
}
- /* the race is on. caller could have timed out immediately after
- we verified the magic, but if so, caller will wait a short time
- for us to copy over the rssi */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pStatsContext->magic = 0;
+
+ /* copy over the rssi */
pAdapter->rssi = rssi;
- /* and notify the caller */
+ /* notify the caller */
complete(&pStatsContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
static void hdd_GetSnrCB(tANI_S8 snr, tANI_U32 staId, void *pContext)
@@ -629,17 +633,19 @@
return;
}
- /* there is a race condition that exists between this callback function
- * and the caller since the caller could time out either before or
- * while this code is executing. we'll assume the timeout hasn't
- * occurred, but we'll verify that right before we save our work
- */
-
pStatsContext = pContext;
pAdapter = pStatsContext->pAdapter;
+
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
+
if ((NULL == pAdapter) || (SNR_CONTEXT_MAGIC != pStatsContext->magic))
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
"%s: Invalid context, pAdapter [%p] magic [%08x]",
__func__, pAdapter, pStatsContext->magic);
@@ -651,14 +657,19 @@
return;
}
- /* the race is on. caller could have timed out immediately after
- * we verified the magic, but if so, caller will wait a short time
- * for us to copy over the snr
- */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pStatsContext->magic = 0;
+
+ /* copy over the snr */
pAdapter->snr = snr;
- /* and notify the caller */
+ /* notify the caller */
complete(&pStatsContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
VOS_STATUS wlan_hdd_get_rssi(hdd_adapter_t *pAdapter, v_S7_t *rssi_value)
@@ -705,24 +716,29 @@
/* request was sent -- wait for the response */
lrc = wait_for_completion_interruptible_timeout(&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_STATS));
- /* either we have a response or we timed out
- either way, first invalidate our magic */
- context.magic = 0;
if (lrc <= 0)
{
- hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while retrieving RSSI ",
+ hddLog(VOS_TRACE_LEVEL_ERROR, "%s: SME %s while retrieving RSSI",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
/* we'll now returned a cached value below */
}
}
+
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
*rssi_value = pAdapter->rssi;
return VOS_STATUS_SUCCESS;
@@ -779,26 +795,29 @@
/* request was sent -- wait for the response */
lrc = wait_for_completion_interruptible_timeout(&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_STATS));
- /* either we have a response or we timed out
- * either way, first invalidate our magic
- */
- context.magic = 0;
if (lrc <= 0)
{
- hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while retrieving SNR ",
+ hddLog(VOS_TRACE_LEVEL_ERROR, "%s: SME %s while retrieving SNR",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- * function could be executing at the same time we are. Of
- * primary concern is if the callback function had already
- * verified the "magic" but hasn't yet set the completion
- * variable. Since the completion variable is on our
- * stack, we'll delay just a bit to make sure the data is
- * still valid if that is the case
- */
- msleep(50);
/* we'll now returned a cached value below */
}
}
+
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
*snr = pAdapter->snr;
return VOS_STATUS_SUCCESS;
@@ -823,16 +842,19 @@
return;
}
- /* there is a race condition that exists between this callback function
- and the caller since the caller could time out either before or
- while this code is executing. we'll assume the timeout hasn't
- occurred, but we'll verify that right before we save our work */
-
pStatsContext = pContext;
pAdapter = pStatsContext->pAdapter;
+
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
+
if ((NULL == pAdapter) || (RSSI_CONTEXT_MAGIC != pStatsContext->magic))
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
"%s: Invalid context, pAdapter [%p] magic [%08x]",
__func__, pAdapter, pStatsContext->magic);
@@ -844,13 +866,19 @@
return;
}
- /* the race is on. caller could have timed out immediately after
- we verified the magic, but if so, caller will wait a short time
- for us to copy over the rssi */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pStatsContext->magic = 0;
+
+ /* copy over the rssi */
pAdapter->rssi = rssi;
- /* and notify the caller */
+ /* notify the caller */
complete(&pStatsContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
@@ -906,24 +934,29 @@
/* request was sent -- wait for the response */
lrc = wait_for_completion_interruptible_timeout(&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_STATS));
- /* either we have a response or we timed out
- either way, first invalidate our magic */
- context.magic = 0;
if (lrc <= 0)
{
- hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while retrieving RSSI ",
+ hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while retrieving RSSI",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
/* we'll now returned a cached value below */
}
}
+
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
*rssi_value = pAdapter->rssi;
return VOS_STATUS_SUCCESS;
@@ -2169,7 +2202,6 @@
static void iw_power_callback_fn (void *pContext, eHalStatus status)
{
struct statsContext *pStatsContext;
- hdd_adapter_t *pAdapter;
if (NULL == pContext)
{
@@ -2179,31 +2211,40 @@
return;
}
- /* there is a race condition that exists between this callback function
- and the caller since the caller could time out either before or
- while this code is executing. we'll assume the timeout hasn't
- occurred, but we'll verify that right before we save our work */
-
pStatsContext = (struct statsContext *)pContext;
- pAdapter = pStatsContext->pAdapter;
- if ((NULL == pAdapter) || (POWER_CONTEXT_MAGIC != pStatsContext->magic))
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
+
+ if (POWER_CONTEXT_MAGIC != pStatsContext->magic)
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
- "%s: Invalid context, pAdapter [%p] magic [%08x]",
- __func__, pAdapter, pStatsContext->magic);
+ "%s: Invalid context, magic [%08x]",
+ __func__, pStatsContext->magic);
if (ioctl_debug)
{
- pr_info("%s: Invalid context, pAdapter [%p] magic [%08x]\n",
- __func__, pAdapter, pStatsContext->magic);
+ pr_info("%s: Invalid context, magic [%08x]\n",
+ __func__, pStatsContext->magic);
}
return;
}
- /* and notify the caller */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pStatsContext->magic = 0;
+
+ /* notify the caller */
complete(&pStatsContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
/* Callback function for tx per hit */
@@ -2243,17 +2284,20 @@
return;
}
- /* there is a race condition that exists between this callback function
- and the caller since the caller could time out either before or
- while this code is executing. we'll assume the timeout hasn't
- occurred, but we'll verify that right before we save our work */
-
pClassAStats = pStats;
pStatsContext = pContext;
pAdapter = pStatsContext->pAdapter;
+
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
+
if ((NULL == pAdapter) || (STATS_CONTEXT_MAGIC != pStatsContext->magic))
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
"%s: Invalid context, pAdapter [%p] magic [%08x]",
__func__, pAdapter, pStatsContext->magic);
@@ -2265,13 +2309,19 @@
return;
}
- /* the race is on. caller could have timed out immediately after
- we verified the magic, but if so, caller will wait a short time
- for us to copy over the stats. do so as a struct copy */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pStatsContext->magic = 0;
+
+ /* copy over the stats. do so as a struct copy */
pAdapter->hdd_stats.ClassA_stat = *pClassAStats;
- /* and notify the caller */
+ /* notify the caller */
complete(&pStatsContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
VOS_STATUS wlan_hdd_get_classAstats(hdd_adapter_t *pAdapter)
@@ -2309,7 +2359,7 @@
if (eHAL_STATUS_SUCCESS != hstatus)
{
hddLog(VOS_TRACE_LEVEL_ERROR,
- "%s: Unable to retrieve Class A statistics ",
+ "%s: Unable to retrieve Class A statistics",
__func__);
/* we'll returned a cached value below */
}
@@ -2318,24 +2368,30 @@
/* request was sent -- wait for the response */
lrc = wait_for_completion_interruptible_timeout(&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_STATS));
- /* either we have a response or we timed out
- either way, first invalidate our magic */
- context.magic = 0;
if (lrc <= 0)
{
hddLog(VOS_TRACE_LEVEL_ERROR,
"%s: SME %s while retrieving Class A statistics",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
}
}
+
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
+ /* either callback updated pAdapter stats or it has cached data */
return VOS_STATUS_SUCCESS;
}
@@ -2360,10 +2416,11 @@
return;
}
- /* there is a race condition that exists between this callback function
- and the caller since the caller could time out either before or
- while this code is executing. we'll assume the timeout hasn't
- occurred, but we'll verify that right before we save our work */
+ /* there is a race condition that exists between this callback
+ function and the caller since the caller could time out either
+ before or while this code is executing. we use a spinlock to
+ serialize these actions */
+ spin_lock(&hdd_context_lock);
pSummaryStats = (tCsrSummaryStatsInfo *)pStats;
pClassAStats = (tCsrGlobalClassAStatsInfo *)( pSummaryStats + 1 );
@@ -2372,6 +2429,7 @@
if ((NULL == pAdapter) || (STATS_CONTEXT_MAGIC != pStatsContext->magic))
{
/* the caller presumably timed out so there is nothing we can do */
+ spin_unlock(&hdd_context_lock);
hddLog(VOS_TRACE_LEVEL_WARN,
"%s: Invalid context, pAdapter [%p] magic [%08x]",
__func__, pAdapter, pStatsContext->magic);
@@ -2383,14 +2441,20 @@
return;
}
- /* the race is on. caller could have timed out immediately after
- we verified the magic, but if so, caller will wait a short time
- for us to copy over the stats. do so as a struct copy */
+ /* context is valid so caller is still waiting */
+
+ /* paranoia: invalidate the magic */
+ pStatsContext->magic = 0;
+
+ /* copy over the stats. do so as a struct copy */
pAdapter->hdd_stats.summary_stat = *pSummaryStats;
pAdapter->hdd_stats.ClassA_stat = *pClassAStats;
- /* and notify the caller */
+ /* notify the caller */
complete(&pStatsContext->completion);
+
+ /* serialization is complete */
+ spin_unlock(&hdd_context_lock);
}
VOS_STATUS wlan_hdd_get_station_stats(hdd_adapter_t *pAdapter)
@@ -2434,24 +2498,31 @@
/* request was sent -- wait for the response */
lrc = wait_for_completion_interruptible_timeout(&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_STATS));
- /* either we have a response or we timed out
- either way, first invalidate our magic */
- context.magic = 0;
+
if (lrc <= 0)
{
hddLog(VOS_TRACE_LEVEL_ERROR,
"%s: SME %s while retrieving statistics",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
}
}
+
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
+ /* either callback updated pAdapter stats or it has cached data */
return VOS_STATUS_SUCCESS;
}
@@ -2650,23 +2721,15 @@
sme_SetDHCPTillPowerActiveFlag(pHddCtx->hHal, TRUE);
if (eHAL_STATUS_PMC_PENDING == status)
{
+ /* request was sent -- wait for the response */
int lrc = wait_for_completion_interruptible_timeout(
&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_POWER));
- context.magic = 0;
+
if (lrc <= 0)
{
hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while requesting fullpower ",
__func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
- /* we'll now returned a cached value below */
}
}
}
@@ -2684,23 +2747,15 @@
iw_power_callback_fn, &context);
if (eHAL_STATUS_PMC_PENDING == status)
{
+ /* request was sent -- wait for the response */
int lrc = wait_for_completion_interruptible_timeout(
&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_POWER));
- context.magic = 0;
if (lrc <= 0)
{
- hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while requesting BMPS ",
- __func__, (0 == lrc) ? "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
- /* we'll now returned a cached value below */
+ hddLog(VOS_TRACE_LEVEL_ERROR,
+ "%s: SME %s while requesting BMPS",
+ __func__, (0 == lrc) ? "timeout" : "interrupt");
}
}
}
@@ -2710,6 +2765,22 @@
"enabled in the cfg");
}
}
+
+ /* either we never sent a request, we sent a request and received a
+ response or we sent a request and timed out. if we never sent a
+ request or if we sent a request and got a response, we want to
+ clear the magic out of paranoia. if we timed out there is a
+ race condition such that the callback function could be
+ executing at the same time we are. of primary concern is if the
+ callback function had already verified the "magic" but had not
+ yet set the completion variable when a timeout occurred. we
+ serialize these activities by invalidating the magic while
+ holding a shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
return VOS_STATUS_SUCCESS;
}
@@ -3820,29 +3891,33 @@
status = sme_RequestFullPower(WLAN_HDD_GET_HAL_CTX(pAdapter),
iw_power_callback_fn, &context,
eSME_FULL_PWR_NEEDED_BY_HDD);
- if(eHAL_STATUS_PMC_PENDING == status)
+ if (eHAL_STATUS_PMC_PENDING == status)
{
int lrc = wait_for_completion_interruptible_timeout(
&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_POWER));
- context.magic = 0;
+
if (lrc <= 0)
{
- hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while "
- "requesting fullpower ",
- __func__, (0 == lrc) ?
- "timeout" : "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
- /* we'll now returned a cached value below */
+ hddLog(VOS_TRACE_LEVEL_ERROR,
+ "%s: SME %s while requesting fullpower",
+ __func__, (0 == lrc) ?
+ "timeout" : "interrupt");
}
}
+ /* either we have a response or we timed out. if we timed
+ out there is a race condition such that the callback
+ function could be executing at the same time we are. of
+ primary concern is if the callback function had already
+ verified the "magic" but had not yet set the completion
+ variable when a timeout occurred. we serialize these
+ activities by invalidating the magic while holding a
+ shared spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
hddLog(LOGE, "iwpriv Full Power completed");
break;
}
@@ -3864,29 +3939,33 @@
status = sme_RequestBmps(WLAN_HDD_GET_HAL_CTX(pAdapter),
iw_power_callback_fn, &context);
- if(eHAL_STATUS_PMC_PENDING == status)
+ if (eHAL_STATUS_PMC_PENDING == status)
{
int lrc = wait_for_completion_interruptible_timeout(
&context.completion,
msecs_to_jiffies(WLAN_WAIT_TIME_POWER));
- context.magic = 0;
if (lrc <= 0)
{
- hddLog(VOS_TRACE_LEVEL_ERROR,"%s: SME %s while "
- "requesting BMPS",
- __func__, (0 == lrc) ? "timeout" :
- "interrupt");
- /* there is a race condition such that the callback
- function could be executing at the same time we are. of
- primary concern is if the callback function had already
- verified the "magic" but hasn't yet set the completion
- variable. Since the completion variable is on our
- stack, we'll delay just a bit to make sure the data is
- still valid if that is the case */
- msleep(50);
- /* we'll now returned a cached value below */
+ hddLog(VOS_TRACE_LEVEL_ERROR,
+ "%s: SME %s while requesting BMPS",
+ __func__, (0 == lrc) ? "timeout" :
+ "interrupt");
}
}
+ /* either we have a response or we timed out. if we
+ timed out there is a race condition such that the
+ callback function could be executing at the same
+ time we are. of primary concern is if the callback
+ function had already verified the "magic" but had
+ not yet set the completion variable when a timeout
+ occurred. we serialize these activities by
+ invalidating the magic while holding a shared
+ spinlock which will cause us to block if the
+ callback is currently executing */
+ spin_lock(&hdd_context_lock);
+ context.magic = 0;
+ spin_unlock(&hdd_context_lock);
+
hddLog(LOGE, "iwpriv Request BMPS completed");
break;
}