Re: [openib-general] [PATCH 5/6] iser RDMA CM (CMA) and IB verbsinteraction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sean Hefty wrote:
+static int iser_free_device_ib_res(struct iser_device *device)
+{
+	BUG_ON(device->mr == NULL);
+
+	tasklet_kill(&device->cq_tasklet);
+
+	(void)ib_dereg_mr(device->mr);
+	(void)ib_destroy_cq(device->cq);
+	(void)ib_dealloc_pd(device->pd);
+
+	device->mr = NULL;
+	device->cq = NULL;
+	device->pd = NULL;
+	return 0;
+}

Can you eliminate the return code?

Yes

+static int iser_free_ib_conn_res(struct iser_conn *ib_conn)
+{
+	BUG_ON(ib_conn == NULL);
+
+	iser_err("freeing conn %p cma_id %p fmr pool %p qp %p\n",
+		 ib_conn, ib_conn->cma_id,
+		 ib_conn->fmr_pool, ib_conn->qp);
+
+	/* qp is created only once both addr & route are resolved */
+	if (ib_conn->fmr_pool != NULL)
+		ib_destroy_fmr_pool(ib_conn->fmr_pool);
+
+	if (ib_conn->qp != NULL)
+		rdma_destroy_qp(ib_conn->cma_id);
+
+	if (ib_conn->cma_id != NULL)
+		rdma_destroy_id(ib_conn->cma_id);

Are the NULL checks needed above?  Neither iser_create_device_ib_res() or
iser_create_ib_conn_res() set the values to NULL if an error occurred.

we are dealing here with connection resources so the (shared among ib conns) device resources are irrelevant. The ib conn struct is kzallec-ed on creation, where later iser_free_ib_conn_res() can be called when only a ***subset*** of the resources was allocated. Examples are instant error from rdma_addr_resolve() or getting ADDR/ROUTE ERROR vs. CONNECT ERROR cma events, in the first three cases only the cma id should be destroyed while on the latter there's a need to destroy the fmr pool and the qp.

+/**
+ * based on the resolved device node GUID see if there already allocated
+ * device for this device. If there's no such, create one.
+ */
+static
+struct iser_device *iser_device_find_by_ib_device(struct rdma_cm_id *cma_id)
+{
+	struct list_head    *p_list;
+	struct iser_device  *device = NULL;
+
+	mutex_lock(&ig.device_list_mutex);
+
+	p_list = ig.device_list.next;
+	while (p_list != &ig.device_list) {
+		device = list_entry(p_list, struct iser_device, ig_list);
+		/* find if there's a match using the node GUID */
+		if (device->ib_device->node_guid == cma_id->device->node_guid)
+			break;
+	}
+
+	if (device == NULL) {
+		device = kzalloc(sizeof *device, GFP_KERNEL);
+		if (device == NULL)
+			goto end;

goto out;  // see below

+		/* assign this device to the device */
+		device->ib_device = cma_id->device;
+		/* init the device and link it into ig device list */
+		if (iser_create_device_ib_res(device)) {
+			kfree(device);
+			device = NULL;
+			goto end;
+		}
+		list_add(&device->ig_list, &ig.device_list);
+	}
+end:
+	BUG_ON(device == NULL);
+	device->refcount++;

out:

OK

Or.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux