[openib-general] hotplug support in mthca

[Michael S. Tsirkin]

Quoting r. Michael S. Tsirkin <mst at mellanox.co.il>:
> Subject: Re: hotplug support in mthca
> 
> Quoting r. Roland Dreier <rdreier at cisco.com>:
> > Subject: Re: hotplug support in mthca
> > 
> >     Michael> How about fixing it by blocking remove_one in uverbs
> >     Michael> until all contexts are closed and device refcount drops
> >     Michael> to 0?
> > 
> > That seems kind of horrible, because there's no guarantee that all the
> > contexts will ever be freed.
> 
> Hmm. Maybe that's an inherent limitation of user-space drivers?
> Isn't this what happens for example if a sysfs file is open?
> 
> How about reporting an event to the application? Would that be sufficient?
> 
> > I think the real answer is to revoke all the contexts that userspace
> > has.  But that doesn't seem trivial to do, which is why I haven't
> > implemented it yet.
> 
> Right, this revoking doesn't sound like 2.6.18 material.
> Isn't just blocking hotplug still better than letting bad things happen?


The following helps avoids crash after hotplug remove.  I think this is at least
better that what we have now, especially if we add to this reporting an event to
the application.
Roland, what do you think?

---

Avoid crash on hotplug remove event by waiting until all users have closed the
device context.

Signed-off-by: Jack Morgenstein <jackm at mellanox.co.il>
Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: src/drivers/infiniband/core/uverbs.h
===================================================================
--- src.orig/drivers/infiniband/core/uverbs.h	2006-08-02 11:14:12.477572000 +0300
+++ src/drivers/infiniband/core/uverbs.h	2006-08-02 12:15:43.950309000 +0300
@@ -42,6 +42,7 @@
 #include <linux/kref.h>
 #include <linux/idr.h>
 #include <linux/mutex.h>
+#include <linux/completion.h>
 
 #include <rdma/ib_verbs.h>
 #include <rdma/ib_user_verbs.h>
@@ -69,6 +70,7 @@
 
 struct ib_uverbs_device {
 	struct kref				ref;
+	struct completion			comp;
 	int					devnum;
 	struct cdev			       *dev;
 	struct class_device		       *class_dev;
Index: src/drivers/infiniband/core/uverbs_main.c
===================================================================
--- src.orig/drivers/infiniband/core/uverbs_main.c	2006-08-02 11:14:12.449574000 +0300
+++ src/drivers/infiniband/core/uverbs_main.c	2006-08-02 12:19:32.537924000 +0300
@@ -122,7 +122,7 @@ static void ib_uverbs_release_dev(struct
 	struct ib_uverbs_device *dev =
 		container_of(ref, struct ib_uverbs_device, ref);
 
-	kfree(dev);
+	complete(&dev->comp);
 }
 
 void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
@@ -740,6 +740,7 @@ static void ib_uverbs_add_one(struct ib_
 		return;
 
 	kref_init(&uverbs_dev->ref);
+	init_completion(&uverbs_dev->comp);
 
 	spin_lock(&map_lock);
 	uverbs_dev->devnum = find_first_zero_bit(dev_map, IB_UVERBS_MAX_DEVICES);
@@ -793,6 +794,8 @@ err_cdev:
 
 err:
 	kref_put(&uverbs_dev->ref, ib_uverbs_release_dev);
+	wait_for_completion(&uverbs_dev->comp);
+	kfree(uverbs_dev);
 	return;
 }
 
@@ -812,7 +815,10 @@ static void ib_uverbs_remove_one(struct 
 	spin_unlock(&map_lock);
 
 	clear_bit(uverbs_dev->devnum, dev_map);
+
 	kref_put(&uverbs_dev->ref, ib_uverbs_release_dev);
+	wait_for_completion(&uverbs_dev->comp);
+	kfree(uverbs_dev);
 }
 
 static struct super_block *uverbs_event_get_sb(struct file_system_type *fs_type, int flags,

-- 
MST