[openib-general] Re: sdp: cant unload ib_ipoib module

[Michael S. Tsirkin]

> > After running ttcp.aio.x multiple times, I am seeing
> > these messages in dmesg:
> > 
> > unregister_netdevice: waiting for ib0 to become free. Usage count = 2
> > 
> > and ipoib cant be unloaded.
> 
Quoting Hal Rosenstock <halr at voltaire.com>:
> This was reported back a while ago. The simplest scenario I have found to
> reproduce this is as follows:

Quoting r. Libor Michalek <limichal at cisco.com>:
> Michael,
> 
>   I remember this problem from the last time Hal mentioned it, I had
> just forgotten about it. I'm almost certain this is being caused by
> a reference counter incremented in the sdp address resolution code
> that is never decremented. In sdp_link.c:do_link_path_lookup() we get
> the route table entry using ip_route_output_key(), which I believe
> takes out a reference, and that reference should be returned using
> ip_rt_put() which we never do.

OK, I grew tired of rebooting and went over sdp_link.c looking for stale
references.
Given how the old code doesnt ever do ip_rt_put in most cases, I dont really
understand why dont more people see this problem.

With the patch below I can unload ipoib after running sdp tests.

Hal, could you verify that this patch helps you, too?

--- 

Drop net_device and rtable references after path lookup is done.

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

Index: linux-2.6.12.2/drivers/infiniband/ulp/sdp/sdp_link.c
===================================================================
--- linux-2.6.12.2.orig/drivers/infiniband/ulp/sdp/sdp_link.c
+++ linux-2.6.12.2/drivers/infiniband/ulp/sdp/sdp_link.c
@@ -327,6 +327,7 @@ static void do_link_path_lookup(void *da
 {
 	struct sdp_path_info *info = data;
 	struct ipoib_dev_priv *priv;
+	struct net_device *loopback = NULL;
 	struct rtable *rt;
 	int counter = 0;
 	int result = 0;
@@ -360,6 +361,7 @@ static void do_link_path_lookup(void *da
 
 	result = ip_route_output_key(&rt, &fl);
 	if (result < 0 || !rt) {
+		rt = NULL;
 		sdp_dbg_warn(NULL, "Error <%d> routing <%08x:%08x> (%d)",
 			     result, info->dst, info->src, info->dif);
 		goto error;
@@ -368,7 +370,6 @@ static void do_link_path_lookup(void *da
 	 * check route flags
 	 */
 	if ((RTCF_MULTICAST|RTCF_BROADCAST) & rt->rt_flags) {
-		ip_rt_put(rt);
 		result = -ENETUNREACH;
 		goto error;
 	}
@@ -402,7 +403,8 @@ static void do_link_path_lookup(void *da
 	 * direct the loopback traffic.
 	 */
 	info->dev = ((rt->u.dst.neighbour->dev->flags & IFF_LOOPBACK) ?
-		     ip_dev_find(rt->rt_src) : rt->u.dst.neighbour->dev);
+		     (loopback = ip_dev_find(rt->rt_src)) :
+		     rt->u.dst.neighbour->dev);
 
 	info->gw  = rt->rt_gateway;
 	info->src = rt->rt_src;     /* true source IP address */
@@ -500,7 +502,9 @@ arp:
 
 	info->flags |= SDP_LINK_F_ARP;
 	queue_delayed_work(link_wq, &info->timer, info->arp_time);
-
+	if (loopback)
+		dev_put(loopback);
+	ip_rt_put(rt);
 	return;
 path:
 	result = sdp_link_path_rec_get(info);
@@ -509,9 +513,15 @@ path:
 		goto error;
 	}
 done:
+	if (loopback)
+		dev_put(loopback);
+	ip_rt_put(rt);
 	return;
 error:
 	sdp_path_info_destroy(info, result);
+	if (loopback)
+		dev_put(loopback);
+	ip_rt_put(rt);
 }
 
 /*


-- 
MST