[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