From 0cf3ff245cc8d285444c3b8742efb6cc49b92fde Mon Sep 17 00:00:00 2001 From: Orhan K AKYILDIZ Date: Thu, 8 Dec 2016 18:19:58 -0800 Subject: qcacld-3.0: Change peer ref count accounting Make sure that peer is deleted at every path where the ref-count is decremented and it reaches zero. Change-Id: Ibaaae1e66855ebbd285b3647f9b862e823b6f335 CRs-Fixed: 1094439 --- core/dp/txrx/ol_tx_classify.c | 8 ++-- core/dp/txrx/ol_txrx.c | 85 +++++++++++++++++++++++++++---------------- core/dp/txrx/ol_txrx.h | 3 +- 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/core/dp/txrx/ol_tx_classify.c b/core/dp/txrx/ol_tx_classify.c index 55a4e930c315..8c72cda2b906 100644 --- a/core/dp/txrx/ol_tx_classify.c +++ b/core/dp/txrx/ol_tx_classify.c @@ -686,6 +686,8 @@ ol_tx_classify_mgmt( * frame to vdev queue. */ if (peer) { + int rcnt; + qdf_mem_copy( &local_mac_addr_aligned.raw[0], dest_addr, OL_TXRX_MAC_ADDR_LEN); @@ -693,13 +695,11 @@ ol_tx_classify_mgmt( if (ol_txrx_peer_find_mac_addr_cmp( mac_addr, &peer->mac_addr) != 0) { - qdf_atomic_dec(&peer->ref_cnt); + rcnt = ol_txrx_peer_unref_delete(peer); QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH, "%s: peer %p peer->ref_cnt %d", - __func__, peer, - qdf_atomic_read - (&peer->ref_cnt)); + __func__, peer, rcnt); peer = NULL; } } diff --git a/core/dp/txrx/ol_txrx.c b/core/dp/txrx/ol_txrx.c index 39932e11e423..9cabed53a1b0 100644 --- a/core/dp/txrx/ol_txrx.c +++ b/core/dp/txrx/ol_txrx.c @@ -2564,6 +2564,7 @@ QDF_STATUS ol_txrx_peer_state_update(struct ol_txrx_pdev_t *pdev, enum ol_txrx_peer_state state) { struct ol_txrx_peer_t *peer; + int peer_ref_cnt; if (qdf_unlikely(!pdev)) { TXRX_PRINT(TXRX_PRINT_LEVEL_ERR, "Pdev is NULL"); @@ -2589,10 +2590,11 @@ QDF_STATUS ol_txrx_peer_state_update(struct ol_txrx_pdev_t *pdev, "%s: no state change, returns directly\n", __func__); #endif - qdf_atomic_dec(&peer->ref_cnt); + peer_ref_cnt = ol_txrx_peer_unref_delete(peer); QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH, - "%s: peer %p peer->ref_cnt %d", __func__, peer, - qdf_atomic_read(&peer->ref_cnt)); + "%s: peer %p peer->ref_cnt %d", + __func__, peer, peer_ref_cnt); + return QDF_STATUS_SUCCESS; } @@ -2622,13 +2624,20 @@ QDF_STATUS ol_txrx_peer_state_update(struct ol_txrx_pdev_t *pdev, ol_txrx_peer_tid_unpause(peer, tid); } } - qdf_atomic_dec(&peer->ref_cnt); + peer_ref_cnt = ol_txrx_peer_unref_delete(peer); QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH, - "%s: peer %p peer->ref_cnt %d", __func__, peer, - qdf_atomic_read(&peer->ref_cnt)); - /* Set the state after the Pause to avoid the race condiction - with ADDBA check in tx path */ - peer->state = state; + "%s: peer %p peer->ref_cnt %d", + __func__, peer, peer_ref_cnt); + /* + * after ol_txrx_peer_unref_delete, peer object cannot be accessed + * if the return code was 0 + */ + if (peer_ref_cnt) + /* + * Set the state after the Pause to avoid the race condiction + * with ADDBA check in tx path + */ + peer->state = state; return QDF_STATUS_SUCCESS; } @@ -2645,6 +2654,7 @@ ol_txrx_peer_update(ol_txrx_vdev_handle vdev, enum ol_txrx_peer_update_select_t select) { struct ol_txrx_peer_t *peer; + int peer_ref_cnt; peer = ol_txrx_peer_find_hash_find(vdev->pdev, peer_mac, 0, 1); if (!peer) { @@ -2726,11 +2736,11 @@ ol_txrx_peer_update(ol_txrx_vdev_handle vdev, __func__); break; } - } - qdf_atomic_dec(&peer->ref_cnt); + } /* switch */ + peer_ref_cnt = ol_txrx_peer_unref_delete(peer); QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH, - "%s: peer %p peer->ref_cnt %d", __func__, peer, - qdf_atomic_read(&peer->ref_cnt)); + "%s: peer %p peer->ref_cnt %d", + __func__, peer, peer_ref_cnt); } uint8_t @@ -2755,8 +2765,9 @@ ol_txrx_peer_qoscapable_get(struct ol_txrx_pdev_t *txrx_pdev, uint16_t peer_id) return 0; } -void ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer) +int ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer) { + int rc; struct ol_txrx_vdev_t *vdev; struct ol_txrx_pdev_t *pdev; int i; @@ -2768,16 +2779,29 @@ void ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer) if (NULL == vdev) { TXRX_PRINT(TXRX_PRINT_LEVEL_INFO1, "The vdev is not present anymore\n"); - return; + return -EINVAL; } pdev = vdev->pdev; if (NULL == pdev) { TXRX_PRINT(TXRX_PRINT_LEVEL_INFO1, "The pdev is not present anymore\n"); - return; + return -EINVAL; } + + /* + * Hold the lock all the way from checking if the peer ref count + * is zero until the peer references are removed from the hash + * table and vdev list (if the peer ref count is zero). + * This protects against a new HL tx operation starting to use the + * peer object just after this function concludes it's done being used. + * Furthermore, the lock needs to be held while checking whether the + * vdev's list of peers is empty, to make sure that list is not modified + * concurrently with the empty check. + */ + qdf_spin_lock_bh(&pdev->peer_ref_mutex); + /* * Check for the reference count before deleting the peer * as we noticed that sometimes we are re-entering this @@ -2785,24 +2809,21 @@ void ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer) * (A double-free should never happen, so assert if it does.) */ - if (0 == qdf_atomic_read(&(peer->ref_cnt))) { + rc = qdf_atomic_read(&(peer->ref_cnt)); + if (rc == 0) { + qdf_spin_unlock_bh(&pdev->peer_ref_mutex); TXRX_PRINT(TXRX_PRINT_LEVEL_ERR, "The Peer is not present anymore\n"); qdf_assert(0); - return; + return -EACCES; } - /* - * Hold the lock all the way from checking if the peer ref count - * is zero until the peer references are removed from the hash - * table and vdev list (if the peer ref count is zero). - * This protects against a new HL tx operation starting to use the - * peer object just after this function concludes it's done being used. - * Furthermore, the lock needs to be held while checking whether the - * vdev's list of peers is empty, to make sure that list is not modified - * concurrently with the empty check. + * now decrement rc; this will be the return code. + * 0 : peer deleted + * >0: peer ref removed, but still has other references + * <0: sanity failed - no changes to the state of the peer */ - qdf_spin_lock_bh(&pdev->peer_ref_mutex); + rc--; if (qdf_atomic_dec_and_test(&peer->ref_cnt)) { u_int16_t peer_id; @@ -2899,11 +2920,13 @@ void ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer) qdf_mem_free(peer); } else { - QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH, - "%s: peer %p peer->ref_cnt = %d", __func__, peer, - qdf_atomic_read(&peer->ref_cnt)); qdf_spin_unlock_bh(&pdev->peer_ref_mutex); + QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH, + "%s: peer %p peer->ref_cnt = %d", + __func__, peer, rc); } + + return rc; } /** diff --git a/core/dp/txrx/ol_txrx.h b/core/dp/txrx/ol_txrx.h index d7982c3b629f..004d330060ec 100644 --- a/core/dp/txrx/ol_txrx.h +++ b/core/dp/txrx/ol_txrx.h @@ -39,8 +39,7 @@ */ #define OL_TX_NON_FWD_RESERVE 100 - -void ol_txrx_peer_unref_delete(struct ol_txrx_peer_t *peer); +int ol_txrx_peer_unref_delete(struct ol_txrx_peer_t *peer); /** * ol_tx_desc_pool_size_hl() - allocate tx descriptor pool size for HL systems -- cgit v1.2.3