[ofa-general] [PATCH] Fix racy deadlock in cma
Roland Dreier
rdreier at cisco.com
Wed Oct 3 10:53:58 PDT 2007
I'll leave it to Sean and others who know the cma locking better than
I do to comment on the patch in detail, but a few notes:
- your patch is completely whitespace mangled so it would have to be
applied by hand. please look into configuring your mail client so
that it can send patches without corrupting them.
- patches should be generated so they apply with 'patch -p1', so
rather than what you have:
> --- drivers/infiniband/core/cma.c 2006-12-13 17:14:23.000000000 -0800
> +++ /tmp/cma.c 2007-10-03 00:48:32.000000000 -0700
the paths should be more like
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
- I wonder which tree you generated your patch against: it seems to
be modifying cma_destroy_listen(), but you have the header:
> @@ -624,6 +624,7 @@
and cma_destroy_listen() is nowhere near line 624 in my tree. (And
it would be nice to use the '-p' option of diff to put the function
name there for easier reviewing)
- Your comment doesn't make it clear to me that dropping and
reacquiring the lock is safe; can you explain why nothing else
could come along while the lock is dropped and mess things up?
It seems rdma_destroy_id() has the same pattern, but it's not clear
to me in the code:
mutex_lock(&lock);
if (id_priv->cma_dev) {
mutex_unlock(&lock);
// why can't the device be hot-unplugged here??
switch (rdma_node_get_transport(id->device->node_type)) {
what guarantees that the device does not disappear before it is
dereferenced in the switch statement. This would be a separate bug
but we probably shouldn't introduce another instance of it
(assuming I'm correct).
- R.
More information about the general
mailing list