PERFORCE change 130115 for review

Steve Wise swise at FreeBSD.org
Mon Dec 3 13:27:55 PST 2007


http://perforce.freebsd.org/chv.cgi?CH=130115

Change 130115 by swise at swise:vic10:iwarp on 2007/12/03 21:27:10

	Fixed refcnt logic on cma and core modules.
		- got rid of atomics since we need spinlocks anyway to do conditional waiting.
		- added logic in destroy paths to only cv_wait() if the refcnt is not 0.
		- added logic in deref to broadcast only if refcnt transitions to 0.

Affected files ...

.. //depot/projects/iwarp/sys/contrib/rdma/ib_addr.h#4 edit
.. //depot/projects/iwarp/sys/contrib/rdma/rdma_addr.c#3 edit
.. //depot/projects/iwarp/sys/contrib/rdma/rdma_cma.c#7 edit
.. //depot/projects/iwarp/sys/contrib/rdma/rdma_device.c#4 edit
.. //depot/projects/iwarp/sys/contrib/rdma/rdma_iwcm.c#4 edit
.. //depot/projects/iwarp/sys/sys/linux_compat.h#7 edit

Differences ...

==== //depot/projects/iwarp/sys/contrib/rdma/ib_addr.h#4 (text+ko) ====

@@ -37,7 +37,7 @@
 #define MAX_ADDR_LEN ETHER_ADDR_LEN	/* XXX doesn't support IB! */
 
 struct rdma_addr_client {
-	atomic_t refcount;
+	int refcount;
 	struct cv comp;
 	struct mtx lock;
 };

==== //depot/projects/iwarp/sys/contrib/rdma/rdma_addr.c#3 (text+ko) ====

@@ -94,17 +94,18 @@
 
 void rdma_addr_register_client(struct rdma_addr_client *client)
 {
-	atomic_set(&client->refcount, 1);
 	mtx_init(&client->lock, "rdma_addr client lock", NULL, MTX_DEF);
 	cv_init(&client->comp, "rdma_addr cv");
+	client->refcount = 1;
 }
 EXPORT_SYMBOL(rdma_addr_register_client);
 
 static inline void put_client(struct rdma_addr_client *client)
 {
 	mtx_lock(&client->lock);
-	if (atomic_dec_and_test(&client->refcount))
-		cv_signal(&client->comp);
+	if (--client->refcount == 0) {
+		cv_broadcast(&client->comp);
+	}
 	mtx_unlock(&client->lock);
 }
 
@@ -112,7 +113,10 @@
 {
 	put_client(client);
 	mtx_lock(&client->lock);
-	cv_wait(&client->comp, &client->lock);
+	if (client->refcount) {
+		cv_wait(&client->comp, &client->lock);
+	}
+	mtx_unlock(&client->lock);
 }
 EXPORT_SYMBOL(rdma_addr_unregister_client);
 
@@ -287,7 +291,9 @@
 	req->callback = callback;
 	req->context = context;
 	req->client = client;
-	atomic_inc(&client->refcount);
+	mtx_lock(&client->lock);
+	client->refcount++;
+	mtx_unlock(&client->lock);
 
 	src_in = (struct sockaddr_in *) &req->src_addr;
 	dst_in = (struct sockaddr_in *) &req->dst_addr;
@@ -306,7 +312,9 @@
 		break;
 	default:
 		ret = req->status;
-		atomic_dec(&client->refcount);
+		mtx_lock(&client->lock);
+		client->refcount--;
+		mtx_unlock(&client->lock);
 		free(req, M_DEVBUF);
 		break;
 	}

==== //depot/projects/iwarp/sys/contrib/rdma/rdma_cma.c#7 (text+ko) ====

@@ -138,7 +138,7 @@
 	struct cv		comp;
 	int			refcount;
 	struct cv		wait_remove;
-	atomic_t		dev_remove;
+	int			dev_remove;
 
 	int			backlog;
 	int			timeout_ms;
@@ -367,9 +367,7 @@
 static void cma_deref_id(struct rdma_id_private *id_priv)
 {
 	mtx_lock(&id_priv->lock);
-	printf("%s id %p refcount %d\n", __FUNCTION__, id_priv, id_priv->refcount);
 	if (--id_priv->refcount == 0) {
-		printf("%s cv_bcast %p \n", __FUNCTION__, id_priv);
 		cv_broadcast(&id_priv->comp);
 	}
 	mtx_unlock(&id_priv->lock);
@@ -382,7 +380,7 @@
 
 	mtx_lock(&id_priv->lock);
 	if (id_priv->state == state) {
-		atomic_inc(&id_priv->dev_remove);
+		id_priv->dev_remove++;
 		ret = 0;
 	} else
 		ret = EINVAL;
@@ -393,7 +391,7 @@
 static void cma_enable_remove(struct rdma_id_private *id_priv)
 {
 	mtx_lock(&id_priv->lock);
-	if (atomic_dec_and_test(&id_priv->dev_remove))
+	if (--id_priv->dev_remove == 0)
 		cv_broadcast(&id_priv->wait_remove);
 	mtx_unlock(&id_priv->lock);
 }
@@ -420,9 +418,7 @@
 	mtx_init(&id_priv->lock, "rdma_cm_id_priv", NULL, MTX_DUPOK|MTX_DEF);
 	cv_init(&id_priv->comp, "rdma_cm_id_priv");
 	id_priv->refcount = 1;
-	printf("%s id %p refcount %d\n", __FUNCTION__, id_priv, id_priv->refcount);
 	cv_init(&id_priv->wait_remove, "id priv wait remove");
-	atomic_set(&id_priv->dev_remove, 0);
 	LIST_INIT(&id_priv->listen_list);
 	arc4rand(&id_priv->seq_num, sizeof id_priv->seq_num, 0);
 
@@ -791,7 +787,8 @@
 	cma_deref_id(id_priv);
 	mtx_lock(&id_priv->lock);
 	if (id_priv->refcount)
-		cv_wait_unlock(&id_priv->comp, &id_priv->lock);
+		cv_wait(&id_priv->comp, &id_priv->lock);
+	mtx_unlock(&id_priv->lock);
 
 	free(id_priv, M_DEVBUF);
 }
@@ -1153,7 +1150,9 @@
 		goto out;
 	}
 
-	atomic_inc(&conn_id->dev_remove);
+	mtx_lock(&conn_id->lock);
+	conn_id->dev_remove++;
+	mtx_unlock(&conn_id->lock);
 	mtx_lock(&lock);
 	ret = cma_acquire_dev(conn_id);
 	mtx_unlock(&lock);
@@ -1322,7 +1321,9 @@
 		goto out;
 	}
 	conn_id = container_of(new_cm_id, struct rdma_id_private, id);
-	atomic_inc(&conn_id->dev_remove);
+	mtx_lock(&conn_id->lock);
+	++conn_id->dev_remove;
+	mtx_unlock(&conn_id->lock);
 	conn_id->state = CMA_CONNECT;
 
 	ifa = ifa_ifwithaddr((struct sockaddr *)&iw_event->local_addr);
@@ -1590,7 +1591,9 @@
 	struct rdma_id_private *id_priv = work->id;
 	int destroy = 0;
 
-	atomic_inc(&id_priv->dev_remove);
+	mtx_lock(&id_priv->lock);
+	++id_priv->dev_remove;
+	mtx_unlock(&id_priv->lock);
 	if (!cma_comp_exch(id_priv, work->old_state, work->new_state))
 		goto out;
 
@@ -1697,7 +1700,6 @@
 
 	mtx_lock(&id_priv->lock);
 	id_priv->refcount++;
-	printf("%s id %p refcount %d\n", __FUNCTION__, id_priv, id_priv->refcount);
 	mtx_unlock(&id_priv->lock);
 #ifdef IB_SUPPORTED
 	switch (rdma_node_get_transport(id->device->node_type)) {
@@ -1773,7 +1775,9 @@
 	struct rdma_cm_event event;
 
 	memset(&event, 0, sizeof event);
-	atomic_inc(&id_priv->dev_remove);
+	mtx_lock(&id_priv->lock);
+	++id_priv->dev_remove;
+	mtx_unlock(&id_priv->lock);
 
 	/*
 	 * Grab mutex to block rdma_destroy_id() from removing the device while
@@ -1879,7 +1883,6 @@
 
 	mtx_lock(&id_priv->lock);
 	id_priv->refcount++;
-	printf("%s id %p refcount %d\n", __FUNCTION__, id_priv, id_priv->refcount);
 	mtx_unlock(&id_priv->lock);
 	memcpy(&id->route.addr.dst_addr, dst_addr, ip_addr_size(dst_addr));
 	if (cma_any_addr(dst_addr))
@@ -2821,7 +2824,10 @@
 
 	cma_cancel_operation(id_priv, state);
 	mtx_lock(&id_priv->lock);
-	cv_wait(&id_priv->wait_remove, &id_priv->lock);
+	BUG_ON(id_priv->dev_remove < 0);
+	if (id_priv->dev_remove)
+		cv_wait(&id_priv->wait_remove, &id_priv->lock);
+	mtx_unlock(&id_priv->lock);
 
 	/* Check for destruction from another callback. */
 	if (!cma_comp(id_priv, CMA_DEVICE_REMOVAL))
@@ -2849,7 +2855,6 @@
 		LIST_REMOVE(id_priv, list);
 		mtx_lock(&id_priv->lock);
 		id_priv->refcount++;
-	printf("%s id %p refcount %d\n", __FUNCTION__, id_priv, id_priv->refcount);
 		mtx_unlock(&id_priv->lock);
 		mtx_unlock(&lock);
 
@@ -2863,7 +2868,11 @@
 	mtx_unlock(&lock);
 
 	cma_deref_dev(cma_dev);
-	cv_wait(&cma_dev->comp, &cma_dev->lock);
+	mtx_lock(&cma_dev->lock);
+	BUG_ON(cma_dev->refcount < 0);
+	if (cma_dev->refcount)
+		cv_wait(&cma_dev->comp, &cma_dev->lock);
+	mtx_unlock(&cma_dev->lock);
 }
 
 static void cma_remove_one(struct ib_device *device)

==== //depot/projects/iwarp/sys/contrib/rdma/rdma_device.c#4 (text+ko) ====


==== //depot/projects/iwarp/sys/contrib/rdma/rdma_iwcm.c#4 (text+ko) ====

@@ -211,7 +211,9 @@
 {
 	struct iwcm_id_private *cm_id_priv;
 	cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);
+	mtx_lock(&cm_id_priv->lock);
 	atomic_inc(&cm_id_priv->refcount);
+	mtx_unlock(&cm_id_priv->lock);
 }
 
 static void rem_ref(struct iw_cm_id *cm_id)
@@ -432,7 +434,9 @@
 	destroy_cm_id(cm_id);
 
 	mtx_lock(&cm_id_priv->lock);
-	cv_wait(&cm_id_priv->destroy_comp, &cm_id_priv->lock);
+	if (atomic_read(&cm_id_priv->refcount))
+		cv_wait(&cm_id_priv->destroy_comp, &cm_id_priv->lock);
+	mtx_unlock(&cm_id_priv->lock);
 
 	free_cm_id(cm_id_priv);
 }
@@ -950,7 +954,9 @@
 		}
 	}
 
+	mtx_lock(&cm_id_priv->lock);
 	atomic_inc(&cm_id_priv->refcount);
+	mtx_unlock(&cm_id_priv->lock);
 	if (TAILQ_EMPTY(&cm_id_priv->work_list)) {
 		TAILQ_INSERT_TAIL(&cm_id_priv->work_list, work, list);
 		taskqueue_enqueue(iwcm_wq, &work->task);

==== //depot/projects/iwarp/sys/sys/linux_compat.h#7 (text+ko) ====

@@ -153,7 +153,7 @@
 static inline void idr_destroy(struct idr *idp)
 {
         struct idr *i, *tmp;
-        for (i=idp;i;i=tmp) {
+        for (i=idp->next;i;i=tmp) {
 		tmp=(i)->next;
 		free(i, M_TEMP);
 	}


More information about the p4-projects mailing list