[openib-general] [PATCH] Extra kfrees, clean up unregisters, etc ...

[Krishna Kumar]

1. Don't kfree sa_dev twice.
2. Unnecessary kref_put : In failure case, we don't seem to have a
   reference until update_sm_ah is called in success case.
3. Clean up code which looks like a hack (i++ in failure).
4. Too many "i -s", no need to keep recalculating this :-)


5. Potential extra cleanup : I could have set index = 0 instead of
   index = i - s, I kept it this way to be quite identical to existing
   code.

Patch applies with -p1 on trunk directory, on top of my previous patch.

Thanks,

- KK

diff -ruNp trunk/src/linux-kernel/infiniband/core/sa_query.c.org 
trunk/src/linux-kernel/infiniband/core/sa_query.c
--- trunk/src/linux-kernel/infiniband/core/sa_query.c.org       2004-11-05 
11:51:06.000000000 -0800
+++ trunk/src/linux-kernel/infiniband/core/sa_query.c   2004-11-05 13:10:50.000000000 
-0800
@@ -682,6 +682,7 @@ static void ib_sa_add_one(struct ib_devi
 {
        struct ib_sa_device *sa_dev;
        int s, e, i;
+       int index;

        if (device->node_type == IB_NODE_SWITCH)
                s = e = 0;
@@ -703,29 +704,29 @@ static void ib_sa_add_one(struct ib_devi
        sa_dev->start_port = s;
        sa_dev->end_port   = e;

-       for (i = s; i <= e; ++i) {
-               sa_dev->port[i - s].mr       = NULL;
-               sa_dev->port[i - s].sm_ah    = NULL;
-               sa_dev->port[i - s].port_num = i;
-               spin_lock_init(&sa_dev->port[i - s].ah_lock);
+       for (i = s, index = i - s; i <= e; ++i, ++index) {
+               sa_dev->port[index].mr       = NULL;
+               sa_dev->port[index].sm_ah    = NULL;
+               sa_dev->port[index].port_num = i;
+               spin_lock_init(&sa_dev->port[index].ah_lock);

-               sa_dev->port[i - s].agent =
+               sa_dev->port[index].agent =
                        ib_register_mad_agent(device, i, IB_QPT_GSI,
                                              NULL, 0, send_handler,
                                              recv_handler, sa_dev);
-               if (IS_ERR(sa_dev->port[i - s].agent))
+               if (IS_ERR(sa_dev->port[index].agent))
                        goto err;

-               sa_dev->port[i - s].mr = ib_get_dma_mr(sa_dev->port[i - 
s].agent->qp->pd,
-                                                      IB_ACCESS_LOCAL_WRITE);
-               if (IS_ERR(sa_dev->port[i - s].mr)) {
-                       /* Bump i so agent from this iter. is freed */
-                       ++i;
+               sa_dev->port[index].mr =
+                               ib_get_dma_mr(sa_dev->port[index].agent->qp->pd,
+                               IB_ACCESS_LOCAL_WRITE);
+               if (IS_ERR(sa_dev->port[index].mr)) {
+                       ib_unregister_mad_agent(sa_dev->port[index].agent);
                        goto err;
                }

-               INIT_WORK(&sa_dev->port[i - s].update_task,
-                         update_sm_ah, &sa_dev->port[i - s]);
+               INIT_WORK(&sa_dev->port[index].update_task,
+                         update_sm_ah, &sa_dev->port[index]);
        }

        /*
@@ -736,27 +737,20 @@ static void ib_sa_add_one(struct ib_devi
         */

        INIT_IB_EVENT_HANDLER(&sa_dev->event_handler, device, ib_sa_event);
-       if (ib_register_event_handler(&sa_dev->event_handler)) {
-               kfree(sa_dev);
+       if (ib_register_event_handler(&sa_dev->event_handler))
                goto err;
-       }

-       for (i = s; i <= e; ++i)
-               update_sm_ah(&sa_dev->port[i - s]);
+       while (--index >= 0)
+               update_sm_ah(&sa_dev->port[index]);

        ib_set_client_data(device, &sa_client, sa_dev);

        return;

 err:
-       while (--i >= s) {
-               if (sa_dev->port[i - s].mr && !IS_ERR(sa_dev->port[i - s].mr))
-                       ib_dereg_mr(sa_dev->port[i - s].mr);
-
-               if (sa_dev->port[i - s].sm_ah)
-                       kref_put(&sa_dev->port[i].sm_ah->ref, free_sm_ah);
-
-               ib_unregister_mad_agent(sa_dev->port[i - s].agent);
+       while (--index >= 0) {
+               ib_dereg_mr(sa_dev->port[index].mr);
+               ib_unregister_mad_agent(sa_dev->port[index].agent);
        }

        kfree(sa_dev);

_______________________________________________
openib-general mailing list
[EMAIL PROTECTED]
http://openib.org/mailman/listinfo/openib-general

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