[openib-general] commit change to IB CM for module reference fix

Sean Hefty mshefty at ichips.intel.com
Fri Apr 7 11:46:07 PDT 2006


Roland,

This is the commit for the IB CM.  Can you please make sure that this changes 
gets merged upstream?

I'll send a separate update for ib_addr and the rdma_cm.

- Sean

---

Add an owner field to cm_id's so that the CM module can take a reference
on modules that it is about to call back into.  This avoids races where
a module's callback function is still running as the module is unloaded.

This patch puts the passing of THIS_MODULE in inline wrappers, so the
externally visible API remains unchanged.

Signed-off-by: Sean Hefty <sean.hefty at intel.com>


Modified: gen2/trunk/src/linux-kernel/infiniband/core/cm.c
===================================================================
--- gen2/trunk/src/linux-kernel/infiniband/core/cm.c	2006-04-07 18:45:24 UTC 
(rev 6323)
+++ gen2/trunk/src/linux-kernel/infiniband/core/cm.c	2006-04-07 18:51:35 UTC 
(rev 6324)
@@ -118,6 +118,7 @@

  struct cm_id_private {
  	struct ib_cm_id	id;
+	struct module *owner;

  	struct rb_node service_node;
  	struct rb_node sidr_id_node;
@@ -590,9 +591,9 @@
  	ib_send_cm_sidr_rep(&cm_id_priv->id, &param);
  }

-struct ib_cm_id *ib_create_cm_id(struct ib_device *device,
+struct ib_cm_id *__ib_create_cm_id(struct ib_device *device,
  				 ib_cm_handler cm_handler,
-				 void *context)
+				 void *context, struct module *owner)
  {
  	struct cm_id_private *cm_id_priv;
  	int ret;
@@ -601,6 +602,7 @@
  	if (!cm_id_priv)
  		return ERR_PTR(-ENOMEM);

+	cm_id_priv->owner = owner;
  	cm_id_priv->id.state = IB_CM_IDLE;
  	cm_id_priv->id.device = device;
  	cm_id_priv->id.cm_handler = cm_handler;
@@ -621,7 +623,7 @@
  	kfree(cm_id_priv);
  	return ERR_PTR(-ENOMEM);
  }
-EXPORT_SYMBOL(ib_create_cm_id);
+EXPORT_SYMBOL(__ib_create_cm_id);

  static struct cm_work * cm_dequeue_work(struct cm_id_private *cm_id_priv)
  {
@@ -1151,6 +1153,18 @@
  	work->cm_event.private_data = &req_msg->private_data;
  }

+static int invoke_cm_handler(struct cm_id_private *cm_id_priv,
+			     struct ib_cm_event *event)
+{
+	int ret;
+
+	BUG_ON(!try_module_get(cm_id_priv->owner));
+	ret = cm_id_priv->id.cm_handler(&cm_id_priv->id, event);
+	module_put(cm_id_priv->owner);
+
+	return ret;
+}
+
  static void cm_process_work(struct cm_id_private *cm_id_priv,
  			    struct cm_work *work)
  {
@@ -1158,7 +1172,7 @@
  	int ret;

  	/* We will typically only have the current event to report. */
-	ret = cm_id_priv->id.cm_handler(&cm_id_priv->id, &work->cm_event);
+	ret = invoke_cm_handler(cm_id_priv, &work->cm_event);
  	cm_free_work(work);

  	while (!ret && !atomic_add_negative(-1, &cm_id_priv->work_count)) {
@@ -1166,8 +1180,7 @@
  		work = cm_dequeue_work(cm_id_priv);
  		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
  		BUG_ON(!work);
-		ret = cm_id_priv->id.cm_handler(&cm_id_priv->id,
-						&work->cm_event);
+		ret = invoke_cm_handler(cm_id_priv, &work->cm_event);
  		cm_free_work(work);
  	}
  	cm_deref_id(cm_id_priv);
@@ -1357,6 +1370,7 @@
  		goto error2;
  	}

+	cm_id_priv->owner = listen_cm_id_priv->owner;
  	cm_id_priv->id.cm_handler = listen_cm_id_priv->id.cm_handler;
  	cm_id_priv->id.context = listen_cm_id_priv->id.context;
  	cm_id_priv->id.service_id = req_msg->service_id;
@@ -2729,6 +2743,7 @@
  	atomic_inc(&cur_cm_id_priv->refcount);
  	spin_unlock_irqrestore(&cm.lock, flags);

+	cm_id_priv->owner = cur_cm_id_priv->owner;
  	cm_id_priv->id.cm_handler = cur_cm_id_priv->id.cm_handler;
  	cm_id_priv->id.context = cur_cm_id_priv->id.context;
  	cm_id_priv->id.service_id = sidr_req_msg->service_id;
@@ -2897,7 +2912,7 @@
  	cm_event.param.send_status = wc_status;

  	/* No other events can occur on the cm_id at this point. */
-	ret = cm_id_priv->id.cm_handler(&cm_id_priv->id, &cm_event);
+	ret = invoke_cm_handler(cm_id_priv, &cm_event);
  	cm_free_msg(msg);
  	if (ret)
  		ib_destroy_cm_id(&cm_id_priv->id);

Modified: gen2/trunk/src/linux-kernel/infiniband/include/rdma/ib_cm.h
===================================================================
--- gen2/trunk/src/linux-kernel/infiniband/include/rdma/ib_cm.h	2006-04-07 
18:45:24 UTC (rev 6323)
+++ gen2/trunk/src/linux-kernel/infiniband/include/rdma/ib_cm.h	2006-04-07 
18:51:35 UTC (rev 6324)
@@ -292,6 +292,10 @@
  	u32			remote_cm_qpn;  /* 1 unless redirected */
  };

+struct ib_cm_id *__ib_create_cm_id(struct ib_device *device,
+				   ib_cm_handler cm_handler,
+				   void *context, struct module *owner);
+
  /**
   * ib_create_cm_id - Allocate a communication identifier.
   * @device: Device associated with the cm_id.  All related communication will
@@ -303,9 +307,12 @@
   * Communication identifiers are used to track connection states, service
   * ID resolution requests, and listen requests.
   */
-struct ib_cm_id *ib_create_cm_id(struct ib_device *device,
-				 ib_cm_handler cm_handler,
-				 void *context);
+static inline struct ib_cm_id *ib_create_cm_id(struct ib_device *device,
+					       ib_cm_handler cm_handler,
+					       void *context)
+{
+	return __ib_create_cm_id(device, cm_handler, context, THIS_MODULE);
+}

  /**
   * ib_destroy_cm_id - Destroy a connection identifier.

_______________________________________________
openib-commits mailing list
openib-commits at openib.org
http://openib.org/mailman/listinfo/openib-commits




More information about the general mailing list