Bläddra i källkod

qcacld-3.0: fix peer map handler race condition

The existing peer_map_unmap_lock in ol_txrx_peer_find_add_id
does not include call to ol_txrx_peer_unref_delete. The peer addition
handling needs to be atomic with peer reference deletion (in case peer
ref deletion is required).

Move the peer_map_unmap_lock to include ol_txrx_peer_unref_delete.

CRs-Fixed: 1056442
Change-Id: Ica15ea70527f0ea116b960dd7958da73f304288b
Mohit Khanna 8 år sedan
förälder
incheckning
47384bcd39
4 ändrade filer med 66 tillägg och 58 borttagningar
  1. 5 3
      core/dp/txrx/ol_tx_classify.c
  2. 3 1
      core/dp/txrx/ol_tx_queue.c
  3. 23 20
      core/dp/txrx/ol_txrx.c
  4. 35 34
      core/dp/txrx/ol_txrx_peer_find.c

+ 5 - 3
core/dp/txrx/ol_tx_classify.c

@@ -694,9 +694,11 @@ ol_tx_classify_mgmt(
 							mac_addr,
 							&peer->mac_addr) != 0) {
 					qdf_atomic_dec(&peer->ref_cnt);
-					qdf_print("%s: peer %p peer->ref_cnt %d",
-						  __func__, peer,
-						  qdf_atomic_read
+					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));
 					peer = NULL;
 				}

+ 3 - 1
core/dp/txrx/ol_tx_queue.c

@@ -92,7 +92,9 @@ ol_tx_queue_vdev_flush(struct ol_txrx_pdev_t *pdev, struct ol_txrx_vdev_t *vdev)
 				txq = &peer->txqs[i];
 				if (txq->frms) {
 					qdf_atomic_inc(&peer->ref_cnt);
-					qdf_print("%s: peer %p peer->ref_cnt %d",
+					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));

+ 23 - 20
core/dp/txrx/ol_txrx.c

@@ -258,8 +258,9 @@ ol_txrx_find_peer_by_addr_and_vdev(ol_txrx_pdev_handle pdev,
 		return NULL;
 	*peer_id = peer->local_id;
 	qdf_atomic_dec(&peer->ref_cnt);
-	qdf_print("%s: peer %p peer->ref_cnt %d", __func__, peer,
-		  qdf_atomic_read(&peer->ref_cnt));
+	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));
 	return peer;
 }
 
@@ -314,8 +315,9 @@ ol_txrx_peer_handle ol_txrx_find_peer_by_addr(ol_txrx_pdev_handle pdev,
 		return NULL;
 	*peer_id = peer->local_id;
 	qdf_atomic_dec(&peer->ref_cnt);
-	qdf_print("%s: peer %p peer->ref_cnt %d", __func__, peer,
-		  qdf_atomic_read(&peer->ref_cnt));
+	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));
 	return peer;
 }
 
@@ -2155,16 +2157,14 @@ ol_txrx_peer_attach(ol_txrx_vdev_handle vdev, uint8_t *peer_mac_addr)
 
 	/* keep one reference for ol_rx_peer_map_handler */
 	qdf_atomic_inc(&peer->ref_cnt);
-	qdf_print("%s: peer %p peer->ref_cnt %d", __func__, peer,
-		  qdf_atomic_read(&peer->ref_cnt));
 
 	peer->valid = 1;
 
 	ol_txrx_peer_find_hash_add(pdev, peer);
 
-	TXRX_PRINT(TXRX_PRINT_LEVEL_INFO2,
-		   "vdev %p created peer %p (%02x:%02x:%02x:%02x:%02x:%02x)\n",
-		   vdev, peer,
+	QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH,
+		   "vdev %p created peer %p ref_cnt %d (%02x:%02x:%02x:%02x:%02x:%02x)\n",
+		   vdev, peer, qdf_atomic_read(&peer->ref_cnt),
 		   peer->mac_addr.raw[0], peer->mac_addr.raw[1],
 		   peer->mac_addr.raw[2], peer->mac_addr.raw[3],
 		   peer->mac_addr.raw[4], peer->mac_addr.raw[5]);
@@ -2554,8 +2554,9 @@ QDF_STATUS ol_txrx_peer_state_update(struct ol_txrx_pdev_t *pdev,
 			   __func__);
 #endif
 		qdf_atomic_dec(&peer->ref_cnt);
-		qdf_print("%s: peer %p peer->ref_cnt %d", __func__, peer,
-			  qdf_atomic_read(&peer->ref_cnt));
+		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));
 		return QDF_STATUS_SUCCESS;
 	}
 
@@ -2586,8 +2587,9 @@ QDF_STATUS ol_txrx_peer_state_update(struct ol_txrx_pdev_t *pdev,
 		}
 	}
 	qdf_atomic_dec(&peer->ref_cnt);
-	qdf_print("%s: peer %p peer->ref_cnt %d", __func__, peer,
-		  qdf_atomic_read(&peer->ref_cnt));
+	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;
@@ -2690,8 +2692,9 @@ ol_txrx_peer_update(ol_txrx_vdev_handle vdev,
 	}
 	}
 	qdf_atomic_dec(&peer->ref_cnt);
-	qdf_print("%s: peer %p peer->ref_cnt %d", __func__, peer,
-		  qdf_atomic_read(&peer->ref_cnt));
+	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));
 }
 
 uint8_t
@@ -2768,7 +2771,7 @@ void ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer)
 		u_int16_t peer_id;
 
 		TXRX_PRINT(TXRX_PRINT_LEVEL_ERR,
-			   "Deleting peer %p (%02x:%02x:%02x:%02x:%02x:%02x)\n",
+			   "Deleting peer %p (%02x:%02x:%02x:%02x:%02x:%02x)",
 			   peer,
 			   peer->mac_addr.raw[0], peer->mac_addr.raw[1],
 			   peer->mac_addr.raw[2], peer->mac_addr.raw[3],
@@ -2817,7 +2820,7 @@ void ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer)
 				TXRX_PRINT(TXRX_PRINT_LEVEL_ERR,
 					   "%s: deleting vdev object %p "
 					   "(%02x:%02x:%02x:%02x:%02x:%02x)"
-					   " - its last peer is done\n",
+					   " - its last peer is done",
 					   __func__, vdev,
 					   vdev->mac_addr.raw[0],
 					   vdev->mac_addr.raw[1],
@@ -2860,9 +2863,9 @@ void ol_txrx_peer_unref_delete(ol_txrx_peer_handle peer)
 
 		qdf_mem_free(peer);
 	} else {
-		TXRX_PRINT(TXRX_PRINT_LEVEL_ERR,
-			   "%s: peer %p peer->ref_cnt = %d\n", __func__, peer,
-			    qdf_atomic_read(&peer->ref_cnt));
+		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);
 	}
 }

+ 35 - 34
core/dp/txrx/ol_txrx_peer_find.c

@@ -181,9 +181,10 @@ struct ol_txrx_peer_t *ol_txrx_peer_vdev_find_hash(struct ol_txrx_pdev_t *pdev,
 			/* found it - increment the ref count before releasing
 			   the lock */
 			qdf_atomic_inc(&peer->ref_cnt);
-			qdf_print("%s: peer %p peer->ref_cnt %d",
-				__func__, peer,
-				qdf_atomic_read(&peer->ref_cnt));
+			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);
 			return peer;
 		}
@@ -217,9 +218,10 @@ struct ol_txrx_peer_t *ol_txrx_peer_find_hash_find(struct ol_txrx_pdev_t *pdev,
 			   releasing the lock */
 			qdf_atomic_inc(&peer->ref_cnt);
 			qdf_spin_unlock_bh(&pdev->peer_ref_mutex);
-			qdf_print("%s: peer %p peer->ref_cnt %d",
-				__func__, peer,
-				  qdf_atomic_read(&peer->ref_cnt));
+			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));
 			return peer;
 		}
 	}
@@ -284,10 +286,11 @@ void ol_txrx_peer_find_hash_erase(struct ol_txrx_pdev_t *pdev)
 				 */
 				qdf_atomic_init(&peer->ref_cnt); /* set to 0 */
 				qdf_atomic_inc(&peer->ref_cnt); /* incr to 1 */
-				TXRX_PRINT(TXRX_PRINT_LEVEL_ERR,
-					   "%s: Delete Peer %p ref_cnt %d\n", __func__,
-					   peer,
-					   qdf_atomic_read(&peer->ref_cnt));
+				QDF_TRACE(QDF_MODULE_ID_TXRX,
+					 QDF_TRACE_LEVEL_INFO_HIGH,
+					 "%s: Delete Peer %p ref_cnt %d\n",
+					 __func__, peer,
+					 qdf_atomic_read(&peer->ref_cnt));
 				ol_txrx_peer_unref_delete(peer);
 			}
 		}
@@ -337,18 +340,19 @@ ol_txrx_peer_find_add_id(struct ol_txrx_pdev_t *pdev,
 	peer =
 		ol_txrx_peer_find_hash_find(pdev, peer_mac_addr,
 					    1 /* is aligned */, 0);
-	if (!peer) {
+
+	if (!peer || peer_id == HTT_INVALID_PEER) {
 		/*
 		 * Currently peer IDs are assigned for vdevs as well as peers.
 		 * If the peer ID is for a vdev, then we will fail to find a
 		 * peer with a matching MAC address.
 		 */
 		TXRX_PRINT(TXRX_PRINT_LEVEL_ERR,
-			"%s: peer not found for ID %d\n", __func__, peer_id);
+			  "%s: peer not found or peer ID is %d invalid",
+			  __func__, peer_id);
 		return;
 	}
 
-
 	qdf_spin_lock(&pdev->peer_map_unmap_lock);
 
 	/* peer's ref count was already incremented by
@@ -370,11 +374,10 @@ ol_txrx_peer_find_add_id(struct ol_txrx_pdev_t *pdev,
 		if (peer->peer_ids[i] == HTT_INVALID_PEER) {
 			peer->peer_ids[i] = peer_id;
 			status = 0;
+			break;
 		}
 	}
 
-	qdf_spin_unlock(&pdev->peer_map_unmap_lock);
-
 	/*
 	 * remove the reference added in ol_txrx_peer_find_hash_find.
 	 * the reference for the first peer id is already added in
@@ -382,22 +385,18 @@ ol_txrx_peer_find_add_id(struct ol_txrx_pdev_t *pdev,
 	 * Riva/Pronto has one peer id for each peer.
 	 * Peregrine/Rome has two peer id for each peer.
 	 */
-	if (del_peer_ref) {
-		TXRX_PRINT(TXRX_PRINT_LEVEL_ERR,
-		   "%s: peer %p ID %d peer_id_ref_cnt %d peer->ref_cnt %d\n",
-		   __func__, peer, peer_id,
-		   qdf_atomic_read(&pdev->
-					peer_id_to_obj_map[peer_id].
-					peer_id_ref_cnt),
-		   qdf_atomic_read(&peer->ref_cnt));
-		/* Keeping ol_txrx_peer_unref_delete outside the
-		 * peer_map_unmap_spinlock as the unref function is protected by
-		 * peer_ref_mutex
-		 */
+	if (del_peer_ref)
 		ol_txrx_peer_unref_delete(peer);
 
-	}
+	qdf_spin_unlock(&pdev->peer_map_unmap_lock);
 
+	QDF_TRACE(QDF_MODULE_ID_TXRX, QDF_TRACE_LEVEL_INFO_HIGH,
+	   "%s: peer %p ID %d peer_id[%d] peer_id_ref_cnt %d peer->ref_cnt %d",
+	   __func__, peer, peer_id, i,
+	   qdf_atomic_read(&pdev->
+				peer_id_to_obj_map[peer_id].
+				peer_id_ref_cnt),
+	   qdf_atomic_read(&peer->ref_cnt));
 
 	if (status) {
 		/* TBDXXX: assert for now */
@@ -519,6 +518,8 @@ void ol_rx_peer_unmap_handler(ol_txrx_pdev_handle pdev, uint16_t peer_id)
 		return;
 	}
 
+	qdf_spin_lock(&pdev->peer_map_unmap_lock);
+
 	peer = pdev->peer_id_to_obj_map[peer_id].peer;
 
 	if (peer == NULL) {
@@ -527,11 +528,10 @@ void ol_rx_peer_unmap_handler(ol_txrx_pdev_handle pdev, uint16_t peer_id)
 	 * If the peer ID is for a vdev, then the peer pointer stored
 	 * in peer_id_to_obj_map will be NULL.
 	 */
+		qdf_spin_unlock(&pdev->peer_map_unmap_lock);
 		return;
 	}
 
-	qdf_spin_lock(&pdev->peer_map_unmap_lock);
-
 	if (qdf_atomic_dec_and_test
 		(&pdev->peer_id_to_obj_map[peer_id].peer_id_ref_cnt)) {
 		pdev->peer_id_to_obj_map[peer_id].peer = NULL;
@@ -547,7 +547,7 @@ void ol_rx_peer_unmap_handler(ol_txrx_pdev_handle pdev, uint16_t peer_id)
 	qdf_spin_unlock(&pdev->peer_map_unmap_lock);
 
 	TXRX_PRINT(TXRX_PRINT_LEVEL_ERR,
-		   "%s: Remove the ID %d reference to peer %p peer_id_ref_cnt %d\n",
+		   "%s: Remove the ID %d reference to peer %p peer_id_ref_cnt %d",
 		   __func__, peer_id, peer,
 		   qdf_atomic_read
 			(&pdev->peer_id_to_obj_map[peer_id].peer_id_ref_cnt));
@@ -572,9 +572,10 @@ struct ol_txrx_peer_t *ol_txrx_assoc_peer_find(struct ol_txrx_vdev_t *vdev)
 	    && vdev->last_real_peer->peer_ids[0] != HTT_INVALID_PEER_ID) {
 		qdf_atomic_inc(&vdev->last_real_peer->ref_cnt);
 		peer = vdev->last_real_peer;
-		qdf_print("%s: peer %p peer->ref_cnt %d",
-			  __func__, peer,
-			  qdf_atomic_read
+		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));
 	} else {
 		peer = NULL;