diff options
| author | Jeff Johnson <jjohnson@qca.qualcomm.com> | 2013-12-19 10:14:15 -0800 |
|---|---|---|
| committer | Prakash Dhavali <pdhavali@qca.qualcomm.com> | 2014-01-31 17:28:03 -0800 |
| commit | 33a506e73db327d73ea5528e188c945693d35409 (patch) | |
| tree | 9a2b13c6e325dd88ba1b8575fc4375703f4f8e01 | |
| parent | a610a4ecaf23d47b16d71f9215ffe6be95bdb29c (diff) | |
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: I5342511d0cc52d45a7d32dbdc9fafd75f75598f7
CRs-fixed: 592261
| -rw-r--r-- | CORE/HDD/inc/wlan_hdd_main.h | 36 | ||||
| -rw-r--r-- | CORE/HDD/src/wlan_hdd_hostapd.c | 19 | ||||
| -rw-r--r-- | CORE/HDD/src/wlan_hdd_main.c | 159 | ||||
| -rw-r--r-- | CORE/HDD/src/wlan_hdd_wext.c | 433 |
4 files changed, 404 insertions, 243 deletions
diff --git a/CORE/HDD/inc/wlan_hdd_main.h b/CORE/HDD/inc/wlan_hdd_main.h index be1c3b19b7ff..f726493e2a60 100644 --- a/CORE/HDD/inc/wlan_hdd_main.h +++ b/CORE/HDD/inc/wlan_hdd_main.h @@ -219,6 +219,37 @@ #define HDD_SESSION_ID_ANY 50 //This should be same as CSR_SESSION_ID_ANY typedef v_U8_t tWlanHddMacAddr[HDD_MAC_ADDR_LEN]; +/* + * 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; @@ -226,7 +257,12 @@ struct statsContext 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 #ifdef FEATURE_WLAN_BATCH_SCAN #define HDD_BATCH_SCAN_VERSION (17) diff --git a/CORE/HDD/src/wlan_hdd_hostapd.c b/CORE/HDD/src/wlan_hdd_hostapd.c index dff529a78c4a..6655a297071a 100644 --- a/CORE/HDD/src/wlan_hdd_hostapd.c +++ b/CORE/HDD/src/wlan_hdd_hostapd.c @@ -93,7 +93,6 @@ extern int process_wma_set_command(int sessid, int paramid, #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 * @@ -3278,15 +3277,29 @@ static VOS_STATUS wlan_hdd_get_classAstats_for_station(hdd_adapter_t *pAdapter, { 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 d311e4c4b0da..c89f536ab0f3 100644 --- a/CORE/HDD/src/wlan_hdd_main.c +++ b/CORE/HDD/src/wlan_hdd_main.c @@ -153,6 +153,12 @@ static int wlan_hdd_inited; #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 * @@ -4417,6 +4423,7 @@ static void hdd_GetTsmStatsCB( tAniTrafStrmMetrics tsmMetrics, { struct statsContext *pStatsContext = NULL; hdd_adapter_t *pAdapter = NULL; + if (NULL == pContext) { hddLog(VOS_TRACE_LEVEL_ERROR, @@ -4424,23 +4431,31 @@ static void hdd_GetTsmStatsCB( tAniTrafStrmMetrics tsmMetrics, __func__, pContext); 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, @@ -4451,8 +4466,12 @@ static void hdd_GetTsmStatsCB( tAniTrafStrmMetrics tsmMetrics, pAdapter->tsmStats.UplinkPktCount = tsmMetrics.UplinkPktCount; 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); } static VOS_STATUS hdd_get_tsm_stats(hdd_adapter_t *pAdapter, @@ -4461,20 +4480,25 @@ static VOS_STATUS hdd_get_tsm_stats(hdd_adapter_t *pAdapter, { hdd_station_ctx_t *pHddStaCtx = NULL; eHalStatus hstatus; + VOS_STATUS vstatus = VOS_STATUS_SUCCESS; long lrc; struct statsContext context; hdd_context_t *pHddCtx = NULL; + if (NULL == pAdapter) { hddLog(VOS_TRACE_LEVEL_ERROR, "%s: pAdapter is NULL", __func__); return VOS_STATUS_E_FAULT; } + pHddCtx = WLAN_HDD_GET_CTX(pAdapter); pHddStaCtx = WLAN_HDD_GET_STATION_CTX_PTR(pAdapter); + /* we are connected prepare our callback context */ init_completion(&context.completion); context.pAdapter = pAdapter; context.magic = STATS_CONTEXT_MAGIC; + /* query tsm stats */ hstatus = sme_GetTsmStats(pHddCtx->hHal, hdd_GetTsmStatsCB, pHddStaCtx->conn_info.staId[ 0 ], @@ -4485,43 +4509,51 @@ static VOS_STATUS hdd_get_tsm_stats(hdd_adapter_t *pAdapter, hddLog(VOS_TRACE_LEVEL_ERROR, "%s: Unable to retrieve statistics", __func__); - return hstatus; + 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 */ @@ -8094,13 +8126,6 @@ void hdd_wlan_initial_scan(hdd_adapter_t *pAdapter) 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 @@ -8116,7 +8141,7 @@ struct fullPowerContext --------------------------------------------------------------------------*/ 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); @@ -8129,24 +8154,32 @@ static void hdd_full_power_callback(void *callbackContext, eHalStatus status) 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); } /**--------------------------------------------------------------------------- @@ -8166,8 +8199,8 @@ void hdd_wlan_exit(hdd_context_t *pHddCtx) v_CONTEXT_t pVosContext = pHddCtx->pvosContext; VOS_STATUS vosStatus; struct wiphy *wiphy = pHddCtx->wiphy; - hdd_adapter_t* pAdapter = NULL; - struct fullPowerContext powerContext; + hdd_adapter_t* pAdapter; + struct statsContext powerContext; long lrc; #if defined (QCA_WIFI_2_0) && \ defined (QCA_WIFI_ISOC) @@ -8303,24 +8336,11 @@ void hdd_wlan_exit(hdd_context_t *pHddCtx) 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 @@ -8328,10 +8348,23 @@ void hdd_wlan_exit(hdd_context_t *pHddCtx) 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); } else { diff --git a/CORE/HDD/src/wlan_hdd_wext.c b/CORE/HDD/src/wlan_hdd_wext.c index bde8b54d28ce..bcfea33acdd0 100644 --- a/CORE/HDD/src/wlan_hdd_wext.c +++ b/CORE/HDD/src/wlan_hdd_wext.c @@ -121,11 +121,6 @@ int hdd_setBand_helper(struct net_device *dev, tANI_U8* ptr); 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}, @@ -768,16 +763,19 @@ static void hdd_GetRssiCB( v_S7_t rssi, tANI_U32 staId, void *pContext ) 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); @@ -789,13 +787,19 @@ static void hdd_GetRssiCB( v_S7_t rssi, tANI_U32 staId, void *pContext ) 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) @@ -817,17 +821,19 @@ static void hdd_GetSnrCB(tANI_S8 snr, tANI_U32 staId, void *pContext) 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); @@ -839,14 +845,19 @@ static void hdd_GetSnrCB(tANI_S8 snr, tANI_U32 staId, void *pContext) 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) @@ -893,24 +904,29 @@ VOS_STATUS wlan_hdd_get_rssi(hdd_adapter_t *pAdapter, v_S7_t *rssi_value) /* 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; @@ -967,26 +983,29 @@ VOS_STATUS wlan_hdd_get_snr(hdd_adapter_t *pAdapter, v_S7_t *snr) /* 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; @@ -1011,16 +1030,19 @@ static void hdd_GetRoamRssiCB( v_S7_t rssi, tANI_U32 staId, void *pContext ) 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); @@ -1032,13 +1054,19 @@ static void hdd_GetRoamRssiCB( v_S7_t rssi, tANI_U32 staId, void *pContext ) 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); } @@ -1094,24 +1122,29 @@ VOS_STATUS wlan_hdd_get_roam_rssi(hdd_adapter_t *pAdapter, v_S7_t *rssi_value) /* 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; @@ -2379,7 +2412,6 @@ static int iw_get_range(struct net_device *dev, struct iw_request_info *info, static void iw_power_callback_fn (void *pContext, eHalStatus status) { struct statsContext *pStatsContext; - hdd_adapter_t *pAdapter; if (NULL == pContext) { @@ -2389,31 +2421,40 @@ static void iw_power_callback_fn (void *pContext, eHalStatus status) 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 */ @@ -2453,17 +2494,20 @@ void hdd_GetClassA_statisticsCB(void *pStats, void *pContext) 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); @@ -2475,13 +2519,19 @@ void hdd_GetClassA_statisticsCB(void *pStats, void *pContext) 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) @@ -2520,7 +2570,7 @@ VOS_STATUS wlan_hdd_get_classAstats(hdd_adapter_t *pAdapter) 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 */ } @@ -2529,24 +2579,30 @@ VOS_STATUS wlan_hdd_get_classAstats(hdd_adapter_t *pAdapter) /* 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; } @@ -2571,10 +2627,11 @@ static void hdd_get_station_statisticsCB(void *pStats, void *pContext) 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 ); @@ -2583,6 +2640,7 @@ static void hdd_get_station_statisticsCB(void *pStats, void *pContext) 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); @@ -2594,14 +2652,20 @@ static void hdd_get_station_statisticsCB(void *pStats, void *pContext) 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) @@ -2646,24 +2710,31 @@ VOS_STATUS wlan_hdd_get_station_stats(hdd_adapter_t *pAdapter) /* 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; } @@ -2862,23 +2933,15 @@ VOS_STATUS wlan_hdd_enter_bmps(hdd_adapter_t *pAdapter, int mode) 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 */ } } } @@ -2896,23 +2959,15 @@ VOS_STATUS wlan_hdd_enter_bmps(hdd_adapter_t *pAdapter, int mode) 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"); } } } @@ -2922,6 +2977,22 @@ VOS_STATUS wlan_hdd_enter_bmps(hdd_adapter_t *pAdapter, int mode) "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; } @@ -4336,29 +4407,33 @@ static int iw_setint_getnone(struct net_device *dev, struct iw_request_info *inf 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\n"); break; } @@ -4380,29 +4455,33 @@ static int iw_setint_getnone(struct net_device *dev, struct iw_request_info *inf 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\n"); break; } |
