Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
Thanks, applied. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
>> Can you queue this for 2.6.19 ? Roland, can you pull this patch in for 2.6.19? It's SVN check-in 9273. --- Clarify that rdma_destroy_id cancels outstanding asynchronous operations on the Associated id. Signed-off-by: Or Gerlitz <[EMAIL PROTECTED]> Signed-off-by: Sean Hefty <[EMAIL PROTECTED]> Index: rdma_cm.h === --- rdma_cm.h (revision 9272) +++ rdma_cm.h (revision 9273) @@ -126,6 +126,14 @@ struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler, void *context, enum rdma_port_space ps); +/** + * rdma_destroy_id - Destroys an RDMA identifier. + * + * @id: RDMA identifier. + * + * Note: calling this function has the effect of canceling in-flight + * asynchronous operations associated with the id. + */ void rdma_destroy_id(struct rdma_cm_id *id); /** ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
Or Gerlitz wrote:
> Or Gerlitz wrote:
>> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
>> index 402c63d..b9e22c8 100644
>> --- a/include/rdma/rdma_cm.h
>> +++ b/include/rdma/rdma_cm.h
>> @@ -117,6 +117,14 @@ struct rdma_cm_id {
>> struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
>> void *context, enum rdma_port_space ps);
>>
>> +/**
>> + * rdma_destroy_id - Destroys an RDMA identifier.
>> + *
>> + * @id: RDMA identifier.
>> + *
>> + * Note: calling this function has the effect of canceling in-flight
>> + * asynchronous operations associated with the id.
>> + */
>> void rdma_destroy_id(struct rdma_cm_id *id);
>>
>> /**
>
> Hi Sean,
>
> Can you queue this for 2.6.19 ?
>
> Or.
>
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
Or Gerlitz wrote:
> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> index 402c63d..b9e22c8 100644
> --- a/include/rdma/rdma_cm.h
> +++ b/include/rdma/rdma_cm.h
> @@ -117,6 +117,14 @@ struct rdma_cm_id {
> struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
> void *context, enum rdma_port_space ps);
>
> +/**
> + * rdma_destroy_id - Destroys an RDMA identifier.
> + *
> + * @id: RDMA identifier.
> + *
> + * Note: calling this function has the effect of canceling in-flight
> + * asynchronous operations associated with the id.
> + */
> void rdma_destroy_id(struct rdma_cm_id *id);
>
> /**
Hi Sean,
Can you queue this for 2.6.19 ?
Or.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
Thanks, queued for 2.6.19. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
>ok, thanks for clarifying that, is cancellation allowed only for address >resolution or also for route resolving and/or CM calls? also how about >documenting this? Cancellation is allowed for any asynchronous operation. I will pull in your patch when I get back in the office. Thanks. - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
Sean Hefty wrote:
>> Does this patch protects against the case where an rdma_cm_id is being
>> destructed while address resolution related to the **same** id attaches
>> it to a device?
>>
>> If yes, why does someone destroys this id? is it legal to do so?
>
> Yes - this protects against the user destroying the id while that same id is
> being attached to a device. This is legal. The user may want to cancel
> address
> resolution by destroying the rdma_cm_id.
ok, thanks for clarifying that, is cancellation allowed only for address
resolution or also for route resolving and/or CM calls? also how about
documenting this?
Or.
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 402c63d..b9e22c8 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -117,6 +117,14 @@ struct rdma_cm_id {
struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
void *context, enum rdma_port_space ps);
+/**
+ * rdma_destroy_id - Destroys an RDMA identifier.
+ *
+ * @id: RDMA identifier.
+ *
+ * Note: calling this function has the effect of canceling in-flight
+ * asynchronous operations associated with the id.
+ */
void rdma_destroy_id(struct rdma_cm_id *id);
/**
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
>Does this patch protects against the case where an rdma_cm_id is being >destructed while address resolution related to the **same** id attaches >it to a device? > >If yes, why does someone destroys this id? is it legal to do so? Yes - this protects against the user destroying the id while that same id is being attached to a device. This is legal. The user may want to cancel address resolution by destroying the rdma_cm_id. The issue is that address resolution is asynchronous, with device attachment occurring in the address resolution callback handler. The user isn't aware that the callback handler has been invoked, and may attempt to destroy the rdma_cm_id when this occurs. - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
Sean Hefty wrote: > This closes a window where address resolution can attach an rdma_cm_id > to a device during destruction of the rdma_cm_id. This can result in > the rdma_cm_id remaining in the device list after its memory has been > freed. Sean, Does this patch protects against the case where an rdma_cm_id is being destructed while address resolution related to the **same** id attaches it to a device? If yes, why does someone destroys this id? is it legal to do so? If not, so your patch protects against the case where one id is being destroyed at the same time another id is being attached to the device? thanks, Or. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] [PATCH] for-2.6.19 cma: protect against adding device during destruction
This closes a window where address resolution can attach an rdma_cm_id
to a device during destruction of the rdma_cm_id. This can result in
the rdma_cm_id remaining in the device list after its memory has been
freed.
Signed-off-by: Sean Hefty <[EMAIL PROTECTED]>
---
I generated this patch off the tip of the for-2.6.19 git branch, so
it applies on top of the iWarp changes.
Also, OF is looking at hosting git repositories. Once available,
I will publish the patches there.
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index c54c55a..2964dab 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -279,7 +279,7 @@ static int cma_acquire_dev(struct rdma_i
default:
return -ENODEV;
}
- mutex_lock(&lock);
+
list_for_each_entry(cma_dev, &dev_list, list) {
ret = ib_find_cached_gid(cma_dev->device, &gid,
&id_priv->id.port_num, NULL);
@@ -288,7 +288,6 @@ static int cma_acquire_dev(struct rdma_i
break;
}
}
- mutex_unlock(&lock);
return ret;
}
@@ -712,7 +711,9 @@ void rdma_destroy_id(struct rdma_cm_id *
state = cma_exch(id_priv, CMA_DESTROYING);
cma_cancel_operation(id_priv, state);
+ mutex_lock(&lock);
if (id_priv->cma_dev) {
+ mutex_unlock(&lock);
switch (rdma_node_get_transport(id->device->node_type)) {
case RDMA_TRANSPORT_IB:
if (id_priv->cm_id.ib && !IS_ERR(id_priv->cm_id.ib))
@@ -727,8 +728,8 @@ void rdma_destroy_id(struct rdma_cm_id *
}
mutex_lock(&lock);
cma_detach_from_dev(id_priv);
- mutex_unlock(&lock);
}
+ mutex_unlock(&lock);
cma_release_port(id_priv);
cma_deref_id(id_priv);
@@ -925,7 +926,9 @@ static int cma_req_handler(struct ib_cm_
}
atomic_inc(&conn_id->dev_remove);
+ mutex_lock(&lock);
ret = cma_acquire_dev(conn_id);
+ mutex_unlock(&lock);
if (ret) {
ret = -ENODEV;
cma_release_remove(conn_id);
@@ -1097,7 +1100,9 @@ static int iw_conn_req_handler(struct iw
goto out;
}
+ mutex_lock(&lock);
ret = cma_acquire_dev(conn_id);
+ mutex_unlock(&lock);
if (ret) {
cma_release_remove(conn_id);
rdma_destroy_id(new_cm_id);
@@ -1507,16 +1512,26 @@ static void addr_handler(int status, str
enum rdma_cm_event_type event;
atomic_inc(&id_priv->dev_remove);
- if (!id_priv->cma_dev && !status)
+
+ /*
+* Grab mutex to block rdma_destroy_id() from removing the device while
+* we're trying to acquire it.
+*/
+ mutex_lock(&lock);
+ if (!cma_comp_exch(id_priv, CMA_ADDR_QUERY, CMA_ADDR_RESOLVED)) {
+ mutex_unlock(&lock);
+ goto out;
+ }
+
+ if (!status && !id_priv->cma_dev)
status = cma_acquire_dev(id_priv);
+ mutex_unlock(&lock);
if (status) {
- if (!cma_comp_exch(id_priv, CMA_ADDR_QUERY, CMA_ADDR_BOUND))
+ if (!cma_comp_exch(id_priv, CMA_ADDR_RESOLVED, CMA_ADDR_BOUND))
goto out;
event = RDMA_CM_EVENT_ADDR_ERROR;
} else {
- if (!cma_comp_exch(id_priv, CMA_ADDR_QUERY, CMA_ADDR_RESOLVED))
- goto out;
memcpy(&id_priv->id.route.addr.src_addr, src_addr,
ip_addr_size(src_addr));
event = RDMA_CM_EVENT_ADDR_RESOLVED;
@@ -1740,8 +1755,11 @@ int rdma_bind_addr(struct rdma_cm_id *id
if (!cma_any_addr(addr)) {
ret = rdma_translate_ip(addr, &id->route.addr.dev_addr);
- if (!ret)
+ if (!ret) {
+ mutex_lock(&lock);
ret = cma_acquire_dev(id_priv);
+ mutex_unlock(&lock);
+ }
if (ret)
goto err;
}
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
