rds: srq initialization and cleanup

RDS has the following two problem related to shared receive queues

1) srq initialization:
  When a new IB dev is registered to device_list, the .add methods
of clients in client_list are called to do some initialization work.

For RDS, rds_ib_add_one() is called. srq related things should be
well initialized here since this is the last change before using srq.

However, code only allocates memory and seems hope rds_ib_srqs_init()
to initialize it later. But infact, rds_ib_srqs_init()
is not called if the call path is not insmod of rds_rdma.

2) srq cleanup:
  When removing rds_rdma module, srqs for all rds_ib_device should
be cleaned up. However, code only frees the rds_ib_device.srq memory
and is not cleaning up memory pointed to by pointers embedded inside.
This lead to resource leak.

This patch fixes the above two problems.

Orabug: 21795815

Signed-off-by: Wengang Wang <[email protected]>
Reviewed-by: Chien Yen <[email protected]>
Signed-off-by: Guangyu Sun <[email protected]>
Signed-off-by: Mukesh Kacker <[email protected]>

Author: Wengang-oracle

net/rds/ib.c

@@ -217,7 +217,10 @@ static void rds_ib_dev_free(struct work_struct *work)
 		if (rds_ibdev->pd)
 			ib_dealloc_pd(rds_ibdev->pd);
 	}
-	kfree(rds_ibdev->srq);
+	if (rds_ibdev->srq) {
+		rds_ib_srq_exit(rds_ibdev);
+		kfree(rds_ibdev->srq);
+	}
 
 	list_for_each_entry_safe(i_ipaddr, i_next, &rds_ibdev->ipaddr_list, list) {
 		list_del(&i_ipaddr->list);
@@ -2170,13 +2173,12 @@ void rds_ib_add_one(struct ib_device *device)
 		goto put_dev;
 	}
 
-	rds_ibdev->srq = kmalloc(sizeof(struct rds_ib_srq), GFP_KERNEL);
-	if (!rds_ibdev->srq)
-		goto free_attr;
-
 	INIT_LIST_HEAD(&rds_ibdev->ipaddr_list);
 	INIT_LIST_HEAD(&rds_ibdev->conn_list);
 
+	if (rds_ib_srq_init(rds_ibdev))
+		goto put_dev;
+
 	down_write(&rds_ib_devices_lock);
 	list_add_tail_rcu(&rds_ibdev->list, &rds_ib_devices);
 	up_write(&rds_ib_devices_lock);
@@ -2622,33 +2624,27 @@ int rds_ib_init(void)
 	if (ret)
 		goto out;
 
-	ret = ib_register_client(&rds_ib_client);
-	if (ret)
-		goto out_fmr_exit;
-
 	ret = rds_ib_sysctl_init();
 	if (ret)
-		goto out_ibreg;
+		goto out_fmr_exit;
 
 	ret = rds_ib_recv_init();
 	if (ret)
 		goto out_sysctl;
 
-	ret = rds_ib_srqs_init();
-	if (ret) {
-		printk(KERN_ERR "RDS/IB: Failed to init SRQ\n");
+	ret = ib_register_client(&rds_ib_client);
+	if (ret)
 		goto out_recv;
-	}
 
 	rds_aux_wq = create_singlethread_workqueue("krdsd_aux");
 	if (!rds_aux_wq) {
 		printk(KERN_ERR "RDS/IB: failed to create aux workqueue\n");
-		goto out_srq;
+		goto out_ibreg;
 	}
 
 	ret = rds_trans_register(&rds_ib_transport);
 	if (ret)
-		goto out_srq;
+		goto out_ibreg;
 
 	rds_info_register_func(RDS_INFO_IB_CONNECTIONS, rds_ib_ic_info);
 
@@ -2657,7 +2653,7 @@ int rds_ib_init(void)
 	ret = rds_ib_ip_config_init();
 	if (ret) {
 		printk(KERN_ERR "RDS/IB: failed to init port\n");
-		goto out_srq;
+		goto out_ibreg;
 	}
 
 	rds_ib_ip_failover_groups_init();
@@ -2666,14 +2662,12 @@ int rds_ib_init(void)
 
 	goto out;
 
-out_srq:
-	rds_ib_srqs_exit();
+out_ibreg:
+	rds_ib_unregister_client();
 out_recv:
 	rds_ib_recv_exit();
 out_sysctl:
 	rds_ib_sysctl_exit();
-out_ibreg:
-	rds_ib_unregister_client();
 out_fmr_exit:
 	rds_ib_fmr_exit();
 out:
@@ -2688,7 +2682,6 @@ void rds_ib_exit(void)
 	rds_ib_unregister_client();
 	rds_ib_destroy_nodev_conns();
 	rds_ib_sysctl_exit();
-	rds_ib_srqs_exit();
 	rds_ib_recv_exit();
 	flush_workqueue(rds_aux_wq);
 	destroy_workqueue(rds_aux_wq);

net/rds/ib.h

@@ -535,6 +535,9 @@ extern struct workqueue_struct *rds_aux_wq;
 extern struct rds_transport rds_ib_transport;
 extern void rds_ib_add_one(struct ib_device *device);
 extern void rds_ib_remove_one(struct ib_device *device);
+void rds_ib_srq_exit(struct rds_ib_device *rds_ibdev);
+int rds_ib_srq_init(struct rds_ib_device *rds_ibdev);
+
 struct rds_ib_device *rds_ib_get_client_data(struct ib_device *device);
 void rds_ib_dev_put(struct rds_ib_device *rds_ibdev);
 extern struct ib_client rds_ib_client;
@@ -604,8 +607,6 @@ void rds_ib_fmr_exit(void);
 /* ib_recv.c */
 int rds_ib_recv_init(void);
 void rds_ib_recv_exit(void);
-int rds_ib_srqs_init(void);
-void rds_ib_srqs_exit(void);
 int rds_ib_recv(struct rds_connection *conn);
 int rds_ib_recv_alloc_caches(struct rds_ib_connection *ic);
 void rds_ib_recv_free_caches(struct rds_ib_connection *ic);

net/rds/ib_recv.c

@@ -376,19 +376,23 @@ static int rds_ib_recv_refill_one(struct rds_connection *conn,
 }
 
 static void rds_ib_srq_clear_one(struct rds_ib_srq *srq,
-				struct rds_ib_connection *ic,
 				struct rds_ib_recv_work *recv)
 {
 	if (recv->r_ibinc) {
-		rds_inc_put(&recv->r_ibinc->ii_inc);
+		if (recv->r_ic)
+			rds_inc_put(&recv->r_ibinc->ii_inc);
+		else
+			kmem_cache_free(rds_ib_incoming_slab, recv->r_ibinc);
 		recv->r_ibinc = NULL;
 	}
 	if (recv->r_frag) {
 		ib_dma_unmap_sg(srq->rds_ibdev->dev, &recv->r_frag->f_sg,
 				1, DMA_FROM_DEVICE);
-		rds_ib_frag_free(ic, recv->r_frag);
+		if (recv->r_ic)
+			rds_ib_frag_free(recv->r_ic, recv->r_frag);
+		else
+			kmem_cache_free(rds_ib_frag_slab, recv->r_frag);
 		recv->r_frag = NULL;
-		recv->r_ic = ic;
 		recv->r_posted = 0;
 	}
 }
@@ -1342,7 +1346,7 @@ void rds_ib_srq_refill(struct work_struct *work)
 
 				for (wr = &cur->r_wr; wr; wr = wr->next) {
 					recv = container_of(wr, struct rds_ib_recv_work, r_wr);
-					rds_ib_srq_clear_one(srq, recv->r_ic, recv);
+					rds_ib_srq_clear_one(srq, recv);
 				}
 				printk(KERN_ERR "ib_post_srq_recv failed\n");
 				goto out;
@@ -1360,7 +1364,7 @@ void rds_ib_srq_refill(struct work_struct *work)
 
 			for (wr = &cur->r_wr; wr; wr = wr->next) {
 				recv = container_of(wr, struct rds_ib_recv_work, r_wr);
-				rds_ib_srq_clear_one(srq, recv->r_ic, recv);
+				rds_ib_srq_clear_one(srq, recv);
 			}
 			printk(KERN_ERR "ib_post_srq_recv failed\n");
 			goto out;
@@ -1413,7 +1417,7 @@ static void rds_ib_srq_clear_ring(struct rds_ib_device *rds_ibdev)
 
 	for (i = 0, recv = rds_ibdev->srq->s_recvs;
 		i < rds_ibdev->srq->s_n_wr; i++, recv++)
-			rds_ib_srq_clear_one(rds_ibdev->srq, recv->r_ic, recv);
+			rds_ib_srq_clear_one(rds_ibdev->srq, recv);
 }
 
 
@@ -1507,6 +1511,19 @@ int rds_ib_srq_init(struct rds_ib_device *rds_ibdev)
 		}
 	};
 
+	/* This is called in two paths
+	 * 1) during insmod of rds_rdma module
+	 * 2) rds_rdma module is ready, a new ib_device added to kernel
+	 */
+	if (!rds_ib_srq_enabled)
+		return 0;
+
+	rds_ibdev->srq = kmalloc(sizeof(struct rds_ib_srq), GFP_KERNEL);
+	if (!rds_ibdev->srq) {
+		pr_warn("RDS: allocating srq failed\n");
+		return 1;
+	}
+
 	rds_ibdev->srq->rds_ibdev = rds_ibdev;
 
 	rds_ibdev->srq->s_n_wr =  rds_ib_srq_max_wr - 1;
@@ -1554,23 +1571,6 @@ int rds_ib_srq_init(struct rds_ib_device *rds_ibdev)
 	return 0;
 }
 
-int rds_ib_srqs_init(void)
-{
-	struct rds_ib_device *rds_ibdev;
-	int ret;
-
-	if (!rds_ib_srq_enabled)
-		return 0;
-
-	list_for_each_entry(rds_ibdev, &rds_ib_devices, list) {
-		ret = rds_ib_srq_init(rds_ibdev);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 void rds_ib_srq_exit(struct rds_ib_device *rds_ibdev)
 {
 	int ret;
@@ -1592,14 +1592,3 @@ void rds_ib_srq_exit(struct rds_ib_device *rds_ibdev)
 	rds_ibdev->srq->s_recvs = NULL;
 }
 
-void rds_ib_srqs_exit(void)
-{
-	struct rds_ib_device *rds_ibdev;
-
-	if (!rds_ib_srq_enabled)
-		return;
-
-	list_for_each_entry(rds_ibdev, &rds_ib_devices, list) {
-		rds_ib_srq_exit(rds_ibdev);
-	}
-}