[ewg] [PATCH] non srq panic patch

[Eli Cohen]

On Thu, 2008-06-26 at 09:25 +0300, Or Gerlitz wrote:
> >   
> If this patch fixes a problem which also exists in the mainline kernel, 
> it has to be accepted upstream (to Roland's tree) before submission to 
> the ewg list. If its not relevant to the mainline code, but rather a 
> result of a problem in a patch that exists only in ofed, the correct way 
> to go would be to fix the patch, send it to the general list for review 
> towards upstream acceptance and then  just pull it into ofed.

Or, this patch is only in ofed. Roland chose a different approach for
coping with the need for large memory rings that do not fit in kmalloc
allocations (Roland uses vmalloc).
> 
> Other then that, this change-log comment is totally mysterious and it 
> requires a detailed read of the patch to know what is the problem and 
> what is the fix.
> 
Yes, please provide a more verbose description of the problem. Please
describe what are the steps that lead to this problem. And finally,
please check if the following patch resolves this problem.

>From fc34c4395875bbd73c4c1fc2a8b06bb246c808f6 Mon Sep 17 00:00:00 2001
From: Eli Cohen <eli at mellanox.co.il>
Date: Thu, 26 Jun 2008 10:32:03 +0300
Subject: [PATCH] Fix bug when freeing rx ring memory

When calling ipoib_cm_free_rx_ring for freeing an rx buffer, the current
code frees the memory for priv->cm.rx_vmap_srq_ring. However when not
working in SRQ mode, this function is called once for each connection
and can lead to multiple free of the same buffer.

This bug did not cause any problem that I could see since ipoib_vfree
calls vunmap and kfree on null pointers.
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 41bed52..7e85a1a 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -231,7 +231,8 @@ static void ipoib_cm_free_rx_ring(struct net_device *dev,
 			dev_kfree_skb_any(rx_ring[i].skb);
 		}
 
-	ipoib_vfree(&priv->cm.rx_vmap_srq_ring);
+	if (!ipoib_cm_has_srq(dev))
+		kfree(rx_ring);
 }
 
 static void ipoib_cm_start_rx_drain(struct ipoib_dev_priv *priv)
@@ -1614,12 +1615,13 @@ void ipoib_cm_dev_cleanup(struct net_device *dev)
 	if (ret)
 		ipoib_warn(priv, "ib_destroy_srq failed: %d\n", ret);
 
-	priv->cm.srq = NULL;
 	if (!priv->cm.srq_ring)
 		return;
 
 	ipoib_cm_free_rx_ring(dev, priv->cm.srq_ring);
+	ipoib_vfree(&priv->cm.rx_vmap_srq_ring);
+	ipoib_vfree(&priv->cm.rx_vmap_wr_arr);
 	priv->cm.srq_ring = NULL;
+	priv->cm.srq = NULL;
 
-	ipoib_vfree(&priv->cm.rx_vmap_wr_arr);
 }
-- 
1.5.6