From 23f63dbf2dbc153937b34d60e2e37740ca2ec8dc Mon Sep 17 00:00:00 2001 From: Wengang Wang Date: Mon, 7 Sep 2015 14:12:42 +0800 Subject: [PATCH] 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 Reviewed-by: Chien Yen Signed-off-by: Guangyu Sun Signed-off-by: Mukesh Kacker --- net/rds/ib.c | 37 ++++++++++++----------------- net/rds/ib.h | 5 ++-- net/rds/ib_recv.c | 59 +++++++++++++++++++---------------------------- 3 files changed, 42 insertions(+), 59 deletions(-) diff --git a/net/rds/ib.c b/net/rds/ib.c index 5dc821faf81e9..cc29ab75b7800 100644 --- a/net/rds/ib.c +++ b/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); diff --git a/net/rds/ib.h b/net/rds/ib.h index e8dd067ac1f59..92dfe07f84ac5 100644 --- a/net/rds/ib.h +++ b/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); diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c index 695bec7277c5c..5c87e7653d8b6 100644 --- a/net/rds/ib_recv.c +++ b/net/rds/ib_recv.c @@ -376,19 +376,23 @@ out: } 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); - } -} -- 2.51.0