[openib-general] [PATCH] sa_query: require SA query registration

Sean Hefty sean.hefty at intel.com
Mon Aug 7 15:32:40 PDT 2006


Require registration with SA module, to prevent module text
from going away while sa query callback is still running,
and update all users.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>
Signed-off-by: Sean Hefty <sean.hefty at intel.com>
---
Changes to the previous post include:

* Move struct ib_sa_client definition internal to SA module to
  better encapsulate future extensions.  We can debate whether
  this is good or bad.
* Fix duplicate dereferences on client objects.
* Add registration/unregistration to SRP.

I did not add tracking to cancel queries automatically.  Adding
this wouldn't change the API - it only speeds up unregistration.
I have a separate patch for the util directory (not posted).

Index: ulp/ipoib/ipoib_main.c
===================================================================
--- ulp/ipoib/ipoib_main.c	(revision 8843)
+++ ulp/ipoib/ipoib_main.c	(working copy)
@@ -91,6 +91,8 @@ static struct ib_client ipoib_client = {
 	.remove = ipoib_remove_one
 };
 
+static struct ib_sa_client *ipoib_sa_client;
+
 int ipoib_open(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
@@ -460,7 +462,7 @@ static int path_rec_start(struct net_dev
 	init_completion(&path->done);
 
 	path->query_id =
-		ib_sa_path_rec_get(priv->ca, priv->port,
+		ib_sa_path_rec_get(ipoib_sa_client, priv->ca, priv->port,
 				   &path->pathrec,
 				   IB_SA_PATH_REC_DGID		|
 				   IB_SA_PATH_REC_SGID		|
@@ -1185,12 +1187,21 @@ static int __init ipoib_init_module(void
 		goto err_fs;
 	}
 
+	ipoib_sa_client = ib_sa_register_client();
+	if (IS_ERR(ipoib_sa_client)) {
+		ret = PTR_ERR(ipoib_sa_client);
+		goto err_wq;
+	}
+
 	ret = ib_register_client(&ipoib_client);
 	if (ret)
-		goto err_wq;
+		goto err_sa;
 
 	return 0;
 
+err_sa:
+	ib_sa_unregister_client(ipoib_sa_client);
+
 err_wq:
 	destroy_workqueue(ipoib_workqueue);
 
@@ -1202,6 +1213,7 @@ err_fs:
 
 static void __exit ipoib_cleanup_module(void)
 {
+	ib_sa_unregister_client(ipoib_sa_client);
 	ib_unregister_client(&ipoib_client);
 	ipoib_unregister_debugfs();
 	destroy_workqueue(ipoib_workqueue);
Index: ulp/srp/ib_srp.c
===================================================================
--- ulp/srp/ib_srp.c	(revision 8843)
+++ ulp/srp/ib_srp.c	(working copy)
@@ -103,6 +103,8 @@ static struct ib_client srp_client = {
 	.remove = srp_remove_one
 };
 
+static struct ib_sa_client *srp_sa_client;
+
 static inline struct srp_target_port *host_to_target(struct Scsi_Host *host)
 {
 	return (struct srp_target_port *) host->hostdata;
@@ -2002,10 +2004,17 @@ static int __init srp_init_module(void)
 		return ret;
 	}
 
+	srp_sa_client = ib_sa_register_client();
+	if (IS_ERR(srp_sa_client)) {
+		class_unregister(&srp_class);
+		return PTR_ERR(srp_sa_client);
+	}
+
 	ret = ib_register_client(&srp_client);
 	if (ret) {
 		printk(KERN_ERR PFX "couldn't register IB client\n");
 		class_unregister(&srp_class);
+		ib_sa_unregister_client(srp_sa_client);
 		return ret;
 	}
 
@@ -2014,6 +2023,7 @@ static int __init srp_init_module(void)
 
 static void __exit srp_cleanup_module(void)
 {
+	ib_sa_unregister_client(srp_sa_client);
 	ib_unregister_client(&srp_client);
 	class_unregister(&srp_class);
 }
Index: include/rdma/ib_sa.h
===================================================================
--- include/rdma/ib_sa.h	(revision 8843)
+++ include/rdma/ib_sa.h	(working copy)
@@ -250,11 +250,25 @@ struct ib_sa_service_rec {
 	u64		data64[2];
 };
 
+struct ib_sa_client;
+
+/**
+ * ib_sa_register_client - Register an SA client.
+ */
+struct ib_sa_client *ib_sa_register_client(void);
+
+/**
+ * ib_sa_unregister_client - Deregister an SA client.
+ * @client: Client object to deregister.
+ */
+void ib_sa_unregister_client(struct ib_sa_client *client);
+
 struct ib_sa_query;
 
 void ib_sa_cancel_query(int id, struct ib_sa_query *query);
 
-int ib_sa_path_rec_get(struct ib_device *device, u8 port_num,
+int ib_sa_path_rec_get(struct ib_sa_client *client,
+		       struct ib_device *device, u8 port_num,
 		       struct ib_sa_path_rec *rec,
 		       ib_sa_comp_mask comp_mask,
 		       int timeout_ms, int retries, gfp_t gfp_mask,
@@ -264,7 +278,8 @@ int ib_sa_path_rec_get(struct ib_device 
 		       void *context,
 		       struct ib_sa_query **query);
 
-int ib_sa_mcmember_rec_query(struct ib_device *device, u8 port_num,
+int ib_sa_mcmember_rec_query(struct ib_sa_client *client,
+			     struct ib_device *device, u8 port_num,
 			     u8 method,
 			     struct ib_sa_mcmember_rec *rec,
 			     ib_sa_comp_mask comp_mask,
@@ -275,7 +290,8 @@ int ib_sa_mcmember_rec_query(struct ib_d
 			     void *context,
 			     struct ib_sa_query **query);
 
-int ib_sa_service_rec_query(struct ib_device *device, u8 port_num,
+int ib_sa_service_rec_query(struct ib_sa_client *client,
+			 struct ib_device *device, u8 port_num,
 			 u8 method,
 			 struct ib_sa_service_rec *rec,
 			 ib_sa_comp_mask comp_mask,
@@ -288,6 +304,7 @@ int ib_sa_service_rec_query(struct ib_de
 
 /**
  * ib_sa_mcmember_rec_set - Start an MCMember set query
+ * @client:SA client
  * @device:device to send query on
  * @port_num: port number to send query on
  * @rec:MCMember Record to send in query
@@ -312,7 +329,8 @@ int ib_sa_service_rec_query(struct ib_de
  * cancel the query.
  */
 static inline int
-ib_sa_mcmember_rec_set(struct ib_device *device, u8 port_num,
+ib_sa_mcmember_rec_set(struct ib_sa_client *client,
+		       struct ib_device *device, u8 port_num,
 		       struct ib_sa_mcmember_rec *rec,
 		       ib_sa_comp_mask comp_mask,
 		       int timeout_ms, int retries, gfp_t gfp_mask,
@@ -322,7 +340,7 @@ ib_sa_mcmember_rec_set(struct ib_device 
 		       void *context,
 		       struct ib_sa_query **query)
 {
-	return ib_sa_mcmember_rec_query(device, port_num,
+	return ib_sa_mcmember_rec_query(client, device, port_num,
 					IB_MGMT_METHOD_SET,
 					rec, comp_mask,
 					timeout_ms, retries, gfp_mask, callback,
@@ -331,6 +349,7 @@ ib_sa_mcmember_rec_set(struct ib_device 
 
 /**
  * ib_sa_mcmember_rec_delete - Start an MCMember delete query
+ * @client:SA client
  * @device:device to send query on
  * @port_num: port number to send query on
  * @rec:MCMember Record to send in query
@@ -355,7 +374,8 @@ ib_sa_mcmember_rec_set(struct ib_device 
  * cancel the query.
  */
 static inline int
-ib_sa_mcmember_rec_delete(struct ib_device *device, u8 port_num,
+ib_sa_mcmember_rec_delete(struct ib_sa_client *client,
+			  struct ib_device *device, u8 port_num,
 			  struct ib_sa_mcmember_rec *rec,
 			  ib_sa_comp_mask comp_mask,
 			  int timeout_ms, int retries, gfp_t gfp_mask,
@@ -365,7 +385,7 @@ ib_sa_mcmember_rec_delete(struct ib_devi
 			  void *context,
 			  struct ib_sa_query **query)
 {
-	return ib_sa_mcmember_rec_query(device, port_num,
+	return ib_sa_mcmember_rec_query(client, device, port_num,
 					IB_SA_METHOD_DELETE,
 					rec, comp_mask,
 					timeout_ms, retries, gfp_mask, callback,
Index: core/multicast.c
===================================================================
--- core/multicast.c	(revision 8843)
+++ core/multicast.c	(working copy)
@@ -63,6 +63,7 @@ static struct ib_client mcast_client = {
 	.remove = mcast_remove_one
 };
 
+static struct ib_sa_client	*sa_client;
 static struct ib_event_handler	event_handler;
 static struct workqueue_struct	*mcast_wq;
 static union ib_gid mgid0;
@@ -305,8 +306,8 @@ static int send_join(struct mcast_group 
 	int ret;
 
 	group->last_join = member;
-	ret = ib_sa_mcmember_rec_set(port->dev->device, port->port_num,
-				     &member->multicast.rec,
+	ret = ib_sa_mcmember_rec_set(sa_client, port->dev->device,
+				     port->port_num, &member->multicast.rec,
 				     member->multicast.comp_mask,
 				     retry_timer, retries, GFP_KERNEL,
 				     join_handler, group, &group->query);
@@ -326,7 +327,8 @@ static int send_leave(struct mcast_group
 	rec = group->rec;
 	rec.join_state = leave_state;
 
-	ret = ib_sa_mcmember_rec_delete(port->dev->device, port->port_num, &rec,
+	ret = ib_sa_mcmember_rec_delete(sa_client, port->dev->device,
+					port->port_num, &rec,
 					IB_SA_MCMEMBER_REC_MGID     |
 					IB_SA_MCMEMBER_REC_PORT_GID |
 					IB_SA_MCMEMBER_REC_JOIN_STATE,
@@ -770,18 +772,27 @@ static int __init mcast_init(void)
 	if (!mcast_wq)
 		return -ENOMEM;
 
+	sa_client = ib_sa_register_client();
+	if (IS_ERR(sa_client)) {
+		ret = PTR_ERR(sa_client);
+		goto err1;
+	}
+
 	ret = ib_register_client(&mcast_client);
 	if (ret)
-		goto err;
+		goto err2;
 	return 0;
 
-err:
+err2:
+	ib_sa_unregister_client(sa_client);
+err1:
 	destroy_workqueue(mcast_wq);
 	return ret;
 }
 
 static void __exit mcast_cleanup(void)
 {
+	ib_sa_unregister_client(sa_client);
 	ib_unregister_client(&mcast_client);
 	destroy_workqueue(mcast_wq);
 }
Index: core/sa_query.c
===================================================================
--- core/sa_query.c	(revision 8843)
+++ core/sa_query.c	(working copy)
@@ -33,6 +33,9 @@
  * $Id$
  */
 
+#include <asm/atomic.h>
+
+#include <linux/completion.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
@@ -71,9 +74,15 @@ struct ib_sa_device {
 	struct ib_sa_port port[0];
 };
 
+struct ib_sa_client {
+	atomic_t users;
+	struct completion comp;
+};
+
 struct ib_sa_query {
 	void (*callback)(struct ib_sa_query *, int, struct ib_sa_mad *);
 	void (*release)(struct ib_sa_query *);
+	struct ib_sa_client    *client;
 	struct ib_sa_port      *port;
 	struct ib_mad_send_buf *mad_buf;
 	struct ib_sa_sm_ah     *sm_ah;
@@ -413,6 +422,46 @@ static void ib_sa_event(struct ib_event_
 	}
 }
 
+struct ib_sa_client *ib_sa_register_client()
+{
+	struct ib_sa_client *client;
+
+	client = kmalloc(sizeof *client, GFP_KERNEL);
+	if (!client)
+		return ERR_PTR(-ENOMEM);
+
+	atomic_set(&client->users, 1);
+	init_completion(&client->comp);
+	return client;
+}
+EXPORT_SYMBOL(ib_sa_register_client);
+
+static void ib_sa_client_get(struct ib_sa_query *query,
+			     struct ib_sa_client *client)
+{
+	atomic_inc(&client->users);
+	query->client = client;
+}
+
+static inline void deref_client(struct ib_sa_client *client)
+{
+	if (atomic_dec_and_test(&client->users))
+		complete(&client->comp);
+}
+
+static void ib_sa_client_put(struct ib_sa_query *query)
+{
+	deref_client(query->client);
+}
+
+void ib_sa_unregister_client(struct ib_sa_client *client)
+{
+	deref_client(client);
+	wait_for_completion(&client->comp);
+	kfree(client);
+}
+EXPORT_SYMBOL(ib_sa_unregister_client);
+
 /**
  * ib_sa_cancel_query - try to cancel an SA query
  * @id:ID of query to cancel
@@ -636,7 +685,8 @@ static void ib_sa_path_rec_release(struc
  * error code.  Otherwise it is a query ID that can be used to cancel
  * the query.
  */
-int ib_sa_path_rec_get(struct ib_device *device, u8 port_num,
+int ib_sa_path_rec_get(struct ib_sa_client *client,
+		       struct ib_device *device, u8 port_num,
 		       struct ib_sa_path_rec *rec,
 		       ib_sa_comp_mask comp_mask,
 		       int timeout_ms, int retries, gfp_t gfp_mask,
@@ -677,6 +727,7 @@ int ib_sa_path_rec_get(struct ib_device 
 	mad = query->sa_query.mad_buf->mad;
 	init_mad(mad, agent);
 
+	ib_sa_client_get(&query->sa_query, client);
 	query->sa_query.callback = callback ? ib_sa_path_rec_callback : NULL;
 	query->sa_query.release  = ib_sa_path_rec_release;
 	query->sa_query.port     = port;
@@ -696,6 +747,7 @@ int ib_sa_path_rec_get(struct ib_device 
 
 err2:
 	*sa_query = NULL;
+	ib_sa_client_put(&query->sa_query);
 	ib_free_send_mad(query->sa_query.mad_buf);
 
 err1:
@@ -753,7 +805,8 @@ static void ib_sa_service_rec_release(st
  * error code.  Otherwise it is a request ID that can be used to cancel
  * the query.
  */
-int ib_sa_service_rec_query(struct ib_device *device, u8 port_num, u8 method,
+int ib_sa_service_rec_query(struct ib_sa_client *client,
+			    struct ib_device *device, u8 port_num, u8 method,
 			    struct ib_sa_service_rec *rec,
 			    ib_sa_comp_mask comp_mask,
 			    int timeout_ms, int retries, gfp_t gfp_mask,
@@ -799,6 +852,7 @@ int ib_sa_service_rec_query(struct ib_de
 	mad = query->sa_query.mad_buf->mad;
 	init_mad(mad, agent);
 
+	ib_sa_client_get(&query->sa_query, client);
 	query->sa_query.callback = callback ? ib_sa_service_rec_callback : NULL;
 	query->sa_query.release  = ib_sa_service_rec_release;
 	query->sa_query.port     = port;
@@ -819,6 +873,7 @@ int ib_sa_service_rec_query(struct ib_de
 
 err2:
 	*sa_query = NULL;
+	ib_sa_client_put(&query->sa_query);
 	ib_free_send_mad(query->sa_query.mad_buf);
 
 err1:
@@ -849,7 +904,8 @@ static void ib_sa_mcmember_rec_release(s
 	kfree(container_of(sa_query, struct ib_sa_mcmember_query, sa_query));
 }
 
-int ib_sa_mcmember_rec_query(struct ib_device *device, u8 port_num,
+int ib_sa_mcmember_rec_query(struct ib_sa_client *client,
+			     struct ib_device *device, u8 port_num,
 			     u8 method,
 			     struct ib_sa_mcmember_rec *rec,
 			     ib_sa_comp_mask comp_mask,
@@ -891,6 +947,7 @@ int ib_sa_mcmember_rec_query(struct ib_d
 	mad = query->sa_query.mad_buf->mad;
 	init_mad(mad, agent);
 
+	ib_sa_client_get(&query->sa_query, client);
 	query->sa_query.callback = callback ? ib_sa_mcmember_rec_callback : NULL;
 	query->sa_query.release  = ib_sa_mcmember_rec_release;
 	query->sa_query.port     = port;
@@ -911,6 +968,7 @@ int ib_sa_mcmember_rec_query(struct ib_d
 
 err2:
 	*sa_query = NULL;
+	ib_sa_client_put(&query->sa_query);
 	ib_free_send_mad(query->sa_query.mad_buf);
 
 err1:
@@ -947,6 +1005,7 @@ static void send_handler(struct ib_mad_a
 
         ib_free_send_mad(mad_send_wc->send_buf);
 	kref_put(&query->sm_ah->ref, free_sm_ah);
+	ib_sa_client_put(query);
 	query->release(query);
 }
 
Index: core/cma.c
===================================================================
--- core/cma.c	(revision 8843)
+++ core/cma.c	(working copy)
@@ -61,6 +61,7 @@ static struct ib_client cma_client = {
 	.remove = cma_remove_one
 };
 
+struct ib_sa_client *sa_client;
 static LIST_HEAD(dev_list);
 static LIST_HEAD(listen_any_list);
 static DEFINE_MUTEX(lock);
@@ -1272,7 +1273,7 @@ static int cma_query_ib_route(struct rdm
 	path_rec.pkey = cpu_to_be16(ib_addr_get_pkey(addr));
 	path_rec.numb_path = 1;
 
-	id_priv->query_id = ib_sa_path_rec_get(id_priv->id.device,
+	id_priv->query_id = ib_sa_path_rec_get(sa_client, id_priv->id.device,
 				id_priv->id.port_num, &path_rec,
 				IB_SA_PATH_REC_DGID | IB_SA_PATH_REC_SGID |
 				IB_SA_PATH_REC_PKEY | IB_SA_PATH_REC_NUMB_PATH,
@@ -2367,18 +2368,27 @@ static int cma_init(void)
 	if (!cma_wq)
 		return -ENOMEM;
 
+	sa_client = ib_sa_register_client();
+	if (IS_ERR(sa_client)) {
+		ret = PTR_ERR(sa_client);
+		goto err1;
+	}
+
 	ret = ib_register_client(&cma_client);
 	if (ret)
-		goto err;
+		goto err2;
 	return 0;
 
-err:
+err2:
+	ib_sa_unregister_client(sa_client);
+err1:
 	destroy_workqueue(cma_wq);
 	return ret;
 }
 
 static void cma_cleanup(void)
 {
+	ib_sa_unregister_client(sa_client);
 	ib_unregister_client(&cma_client);
 	destroy_workqueue(cma_wq);
 	idr_destroy(&sdp_ps);





More information about the general mailing list