Date: Mon, 3 Dec 2007 21:27:54 GMT From: Steve Wise <swise@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 130115 for review Message-ID: <200712032127.lB3LRsju022102@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=130115 Change 130115 by swise@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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200712032127.lB3LRsju022102>