[ofa-general] ib_find_gid / ib_find_pkey

Roland Dreier rdreier at cisco.com
Thu May 17 10:19:14 PDT 2007


OK, I applied the ib_find_gid / ib_find_pkey stuff with the following
cleanup on top of it... mostly this is me being picky about
indentation etc, but I think I did fix two bugs: a memory leak if
registering with sysfs fails, and a NULL deref in ib_find_gid if index
is NULL (as the comment says it may be).

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 3f2c619..c084495 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -149,13 +149,13 @@ static int alloc_name(char *name)
 	return 0;
 }
 
-static inline int start_port(struct ib_device *device)
+static int start_port(struct ib_device *device)
 {
 	return (device->node_type == RDMA_NODE_IB_SWITCH) ? 0 : 1;
 }
 
 
-static inline int end_port(struct ib_device *device)
+static int end_port(struct ib_device *device)
 {
 	return (device->node_type == RDMA_NODE_IB_SWITCH) ?
 		0 : device->phys_port_cnt;
@@ -220,7 +220,6 @@ static int add_client_context(struct ib_device *device, struct ib_client *client
 	return 0;
 }
 
-/* read the lengths of pkey,gid tables on each port */
 static int read_port_table_lengths(struct ib_device *device)
 {
 	struct ib_port_attr *tprops = NULL;
@@ -233,42 +232,33 @@ static int read_port_table_lengths(struct ib_device *device)
 
 	num_ports = end_port(device) - start_port(device) + 1;
 
-	device->pkey_tbl_len = kmalloc(sizeof *device->pkey_tbl_len *
-						num_ports, GFP_KERNEL);
-	if (!device->pkey_tbl_len)
-		goto out;
-
-	device->gid_tbl_len = kmalloc(sizeof *device->gid_tbl_len *
-						num_ports, GFP_KERNEL);
-	if (!device->gid_tbl_len)
-		goto err1;
+	device->pkey_tbl_len = kmalloc(sizeof *device->pkey_tbl_len * num_ports,
+				       GFP_KERNEL);
+	device->gid_tbl_len = kmalloc(sizeof *device->gid_tbl_len * num_ports,
+				      GFP_KERNEL);
+	if (!device->pkey_tbl_len || !device->gid_tbl_len)
+		goto err;
 
 	for (port_index = 0; port_index < num_ports; ++port_index) {
 		ret = ib_query_port(device, port_index + start_port(device),
 					tprops);
 		if (ret)
-			goto err2;
+			goto err;
 		device->pkey_tbl_len[port_index] = tprops->pkey_tbl_len;
-		device->gid_tbl_len[port_index] = tprops->gid_tbl_len;
+		device->gid_tbl_len[port_index]  = tprops->gid_tbl_len;
 	}
 
 	ret = 0;
 	goto out;
-err2:
+
+err:
 	kfree(device->gid_tbl_len);
-err1:
 	kfree(device->pkey_tbl_len);
 out:
 	kfree(tprops);
 	return ret;
 }
 
-static inline void free_port_table_lengths(struct ib_device *device)
-{
-	kfree(device->gid_tbl_len);
-	kfree(device->pkey_tbl_len);
-}
-
 /**
  * ib_register_device - Register an IB device with IB core
  * @device:Device to register
@@ -311,6 +301,8 @@ int ib_register_device(struct ib_device *device)
 	if (ret) {
 		printk(KERN_WARNING "Couldn't register device %s with driver model\n",
 		       device->name);
+		kfree(device->gid_tbl_len);
+		kfree(device->pkey_tbl_len);
 		goto out;
 	}
 
@@ -352,7 +344,8 @@ void ib_unregister_device(struct ib_device *device)
 
 	list_del(&device->core_list);
 
-	free_port_table_lengths(device);
+	kfree(device->gid_tbl_len);
+	kfree(device->pkey_tbl_len);
 
 	mutex_unlock(&device_mutex);
 
@@ -672,28 +665,26 @@ EXPORT_SYMBOL(ib_modify_port);
  *   parameter may be NULL.
  */
 int ib_find_gid(struct ib_device *device, union ib_gid *gid,
-			u8 *port_num, u16 *index)
+		u8 *port_num, u16 *index)
 {
 	union ib_gid tmp_gid;
-	int ret, port, i, tbl_len;
+	int ret, port, i;
 
 	for (port = start_port(device); port <= end_port(device); ++port) {
-		tbl_len = device->gid_tbl_len[port - start_port(device)];
-		for (i = 0; i < tbl_len; ++i) {
+		for (i = 0; i < device->gid_tbl_len[port - start_port(device)]; ++i) {
 			ret = ib_query_gid(device, port, i, &tmp_gid);
 			if (ret)
-				goto out;
+				return ret;
 			if (!memcmp(&tmp_gid, gid, sizeof *gid)) {
 				*port_num = port;
-				*index = i;
-				ret = 0;
-				goto out;
+				if (index)
+					*index = i;
+				return 0;
 			}
 		}
 	}
-	ret = -ENOENT;
-out:
-	return ret;
+
+	return -ENOENT;
 }
 EXPORT_SYMBOL(ib_find_gid);
 
@@ -706,27 +697,24 @@ EXPORT_SYMBOL(ib_find_gid);
  * @index: The index into the PKey table where the PKey was found.
  */
 int ib_find_pkey(struct ib_device *device,
-			u8 port_num, u16 pkey, u16 *index)
+		 u8 port_num, u16 pkey, u16 *index)
 {
-	int ret, i, tbl_len;
+	int ret, i;
 	u16 tmp_pkey;
 
-	tbl_len = device->pkey_tbl_len[port_num - start_port(device)];
-	for (i = 0; i < tbl_len; ++i) {
+	tbl_len = ;
+	for (i = 0; i < device->pkey_tbl_len[port_num - start_port(device)]; ++i) {
 		ret = ib_query_pkey(device, port_num, i, &tmp_pkey);
 		if (ret)
-			goto out;
+			return ret;
 
 		if (pkey == tmp_pkey) {
 			*index = i;
-			ret = 0;
-			goto out;
+			return 0;
 		}
 	}
-	ret = -ENOENT;
 
-out:
-	return ret;
+	return -ENOENT;
 }
 EXPORT_SYMBOL(ib_find_pkey);
 
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index a4ae080..0627a6a 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -890,6 +890,8 @@ struct ib_device {
 	spinlock_t                    client_data_lock;
 
 	struct ib_cache               cache;
+	int                          *pkey_tbl_len;
+	int                          *gid_tbl_len;
 
 	u32                           flags;
 
@@ -1043,8 +1045,6 @@ struct ib_device {
 	__be64			     node_guid;
 	u8                           node_type;
 	u8                           phys_port_cnt;
-	int                          *pkey_tbl_len;
-	int                          *gid_tbl_len;
 };
 
 struct ib_client {
@@ -1120,28 +1120,11 @@ int ib_modify_port(struct ib_device *device,
 		   u8 port_num, int port_modify_mask,
 		   struct ib_port_modify *port_modify);
 
-/**
- * ib_find_gid - Returns the port number and GID table index where
- *   a specified GID value occurs.
- * @device: The device to query.
- * @gid: The GID value to search for.
- * @port_num: The port number of the device where the GID value was found.
- * @index: The index into the GID table where the GID was found.  This
- *   parameter may be NULL.
- */
 int ib_find_gid(struct ib_device *device, union ib_gid *gid,
-			u8 *port_num, u16 *index);
+		u8 *port_num, u16 *index);
 
-/**
- * ib_find_pkey - Returns the PKey table index where a specified
- *   PKey value occurs.
- * @device: The device to query.
- * @port_num: The port number of the device to search for the PKey.
- * @pkey: The PKey value to search for.
- * @index: The index into the PKey table where the PKey was found.
- */
 int ib_find_pkey(struct ib_device *device,
-			u8 port_num, u16 pkey, u16 *index);
+		 u8 port_num, u16 pkey, u16 *index);
 
 /**
  * ib_alloc_pd - Allocates an unused protection domain.



More information about the general mailing list