[openib-general] [PATCH] mad.c memory leak

[Michael S. Tsirkin]

Quoting r. Michael S. Tsirkin <mst at mellanox.co.il>:
> Subject: mad.c memory leak
> 
> Sean, I am seeing a memory leak in mad.c: it appears some memory allocated at
> mad.c at line 1766 (ib_mad_recv_done_handler) is not always properly freed.  As
> a result ib_mad_cache can't be destroyed. This happens on x86_64 systems.
> This apparently started happening around the beginning of March.
> I couldn't put my finger on the specific test or change that does this yet,
> could you take a look please?

I'm not sure I see the problem, but I am currently testing the following
cleanup patch: since MAD allocations are already done from cache,
and cache is really fast, it is not worth it to uglify code and post the
response buffer on the receive queue just to save an extra call to alloc/free.

I'll get back to you on whether this fixes the problem.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: linux-2.6.15/drivers/infiniband/core/mad.c
===================================================================
--- linux-2.6.15.orig/drivers/infiniband/core/mad.c	2006-03-15 18:51:36.000000000 +0200
+++ linux-2.6.15/drivers/infiniband/core/mad.c	2006-03-15 19:53:50.000000000 +0200
@@ -1759,15 +1759,10 @@ static void ib_mad_recv_done_handler(str
 {
 	struct ib_mad_qp_info *qp_info;
 	struct ib_mad_private_header *mad_priv_hdr;
-	struct ib_mad_private *recv, *response;
+	struct ib_mad_private *recv, *response = NULL;
 	struct ib_mad_list_head *mad_list;
 	struct ib_mad_agent_private *mad_agent;
 
-	response = kmem_cache_alloc(ib_mad_cache, GFP_KERNEL);
-	if (!response)
-		printk(KERN_ERR PFX "ib_mad_recv_done_handler no memory "
-		       "for response buffer\n");
-
 	mad_list = (struct ib_mad_list_head *)(unsigned long)wc->wr_id;
 	qp_info = mad_list->mad_queue->qp_info;
 	dequeue_mad(mad_list);
@@ -1817,6 +1812,11 @@ local:
 	if (port_priv->device->process_mad) {
 		int ret;
 
+		response = kmem_cache_alloc(ib_mad_cache, GFP_KERNEL);
+		if (!response)
+			printk(KERN_ERR PFX "ib_mad_recv_done_handler "
+			       "no memory for response buffer\n");
+
 		if (!response) {
 			printk(KERN_ERR PFX "No memory for response MAD\n");
 			/*
@@ -1856,13 +1856,10 @@ local:
 	}
 
 out:
-	/* Post another receive request for this QP */
-	if (response) {
-		ib_mad_post_receive_mads(qp_info, response);
-		if (recv)
-			kmem_cache_free(ib_mad_cache, recv);
-	} else
-		ib_mad_post_receive_mads(qp_info, recv);
+	if (response)
+		kmem_cache_free(ib_mad_cache, response);
+
+	ib_mad_post_receive_mads(qp_info, recv);
 }
 
 static void adjust_timeout(struct ib_mad_agent_private *mad_agent_priv)

-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies