Quoting r. Hal Rosenstock <[EMAIL PROTECTED]>:
> Subject: Re: sdp: cant unload ib_ipoib module
> 
> Hi Michael,
> 
> On Tue, 2005-08-09 at 08:46, Michael S. Tsirkin wrote:
> > ip_rt_put now looks right, but it looks like device_put is still done too 
> > early.
> 
> Any idea where it should be done ?

The following should finally fix the dev_put issue.
I think an extra dev_hold on a path lookup is not a big deal, and
helps make the code much simpler. Works fine, for me.
Hal, can you give it a whirl?

Signed-off-by: Michael S. Tsirkin <[EMAIL PROTECTED]>

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,7 +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 net_device *dev = NULL;
        struct rtable *rt;
        int counter = 0;
        int result = 0;
@@ -387,7 +387,7 @@ static void do_link_path_lookup(void *da
         * check for IB device or loopback, the later requires extra
         * handling.
         */
-       if (ARPHRD_INFINIBAND != rt->u.dst.neighbour->dev->type &&
+       if (rt->u.dst.neighbour->dev->type != ARPHRD_INFINIBAND &&
            !(rt->u.dst.neighbour->dev->flags & IFF_LOOPBACK)) {
                result = -ENETUNREACH;
                goto error;
@@ -402,23 +402,28 @@ static void do_link_path_lookup(void *da
         * In case of loopback find a valid IB device on which to
         * direct the loopback traffic.
         */
-       info->dev = ((rt->u.dst.neighbour->dev->flags & IFF_LOOPBACK) ?
-                    (loopback = ip_dev_find(rt->rt_src)) :
-                    rt->u.dst.neighbour->dev);
+       if (rt->u.dst.neighbour->dev->flags & IFF_LOOPBACK)
+               dev = ip_dev_find(rt->rt_src);
+       else {
+               dev = rt->u.dst.neighbour->dev;
+               dev_hold(dev);
+       }
 
        info->gw  = rt->rt_gateway;
        info->src = rt->rt_src;     /* true source IP address */
 
-       if (info->dev->flags & IFF_LOOPBACK)
-               while ((info->dev = dev_get_by_index(++counter))) {
-
-                       dev_put(info->dev);
-                       if (ARPHRD_INFINIBAND == info->dev->type &&
-                           (info->dev->flags & IFF_UP))
+       if (dev->flags & IFF_LOOPBACK) {
+               dev_put(dev);
+               while ((dev = dev_get_by_index(++counter))) {
+                       if (dev->type == ARPHRD_INFINIBAND &&
+                           (dev->flags & IFF_UP))
                                break;
+                       else
+                               dev_put(dev);
                }
+       }
 
-       if (!info->dev) {
+       if (!dev) {
                sdp_dbg_warn(NULL, "No device for IB comm <%s:%08x:%08x>",
                             rt->u.dst.neighbour->dev->name,
                             rt->u.dst.neighbour->dev->flags,
@@ -429,20 +434,20 @@ static void do_link_path_lookup(void *da
        /*
         * lookup local info.
         */
-       priv = info->dev->priv;
+       priv = dev->priv;
 
        info->ca             = priv->ca;
        info->port           = priv->port;
        info->path.pkey      = cpu_to_be16(priv->pkey);
        info->path.numb_path = 1;
 
-       memcpy(&info->path.sgid, info->dev->dev_addr + 4, sizeof(union ib_gid));
+       memcpy(&info->path.sgid, dev->dev_addr + 4, sizeof(union ib_gid));
        /*
         * If the routing device is loopback save the device address of
         * the IB device which was found.
         */
        if (rt->u.dst.neighbour->dev->flags & IFF_LOOPBACK) {
-               memcpy(&info->path.dgid, info->dev->dev_addr + 4,
+               memcpy(&info->path.dgid, dev->dev_addr + 4,
                       sizeof(union ib_gid));
 
                goto path;
@@ -466,10 +471,10 @@ arp:
        arp_send(ARPOP_REQUEST,
                 ETH_P_ARP,
                 info->gw,
-                info->dev,
+                dev,
                 info->src,
                 NULL,
-                info->dev->dev_addr,
+                dev->dev_addr,
                 NULL);
        /*
         * start arp timer if it's not already going.
@@ -498,8 +503,8 @@ arp:
 
        info->flags |= SDP_LINK_F_ARP;
        queue_delayed_work(link_wq, &info->timer, info->arp_time);
-       if (loopback)
-               dev_put(loopback);
+       if (dev)
+               dev_put(dev);
        ip_rt_put(rt);
        return;
 path:
@@ -509,14 +514,14 @@ path:
                goto error;
        }
 done:
-       if (loopback)
-               dev_put(loopback);
+       if (dev)
+               dev_put(dev);
        ip_rt_put(rt);
        return;
 error:
        sdp_path_info_destroy(info, result);
-       if (loopback)
-               dev_put(loopback);
+       if (dev)
+               dev_put(dev);
        ip_rt_put(rt);
 }
 
@@ -690,7 +695,7 @@ static int sdp_link_arp_recv(struct sk_b
 
        arp_hdr = (struct arphdr *)skb->nh.raw;
 
-       if (ARPHRD_INFINIBAND != dev->type ||
+       if (dev->type != ARPHRD_INFINIBAND ||
            (arp_hdr->ar_op != __constant_htons(ARPOP_REPLY) &&
             arp_hdr->ar_op != __constant_htons(ARPOP_REQUEST)))
                goto done;
Index: linux-2.6.12.2/drivers/infiniband/ulp/sdp/sdp_link.h
===================================================================
--- linux-2.6.12.2.orig/drivers/infiniband/ulp/sdp/sdp_link.h
+++ linux-2.6.12.2/drivers/infiniband/ulp/sdp/sdp_link.h
@@ -59,8 +59,6 @@ struct sdp_path_info {
 
        struct work_struct timer;   /* arp request timers. */
 
-       struct net_device     *dev; /* ipoib device */
-
        struct sdp_path_info  *next; /* next element in path list. */
        struct sdp_path_info **pext; /* previous next element in path list */
 


-- 
MST
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to