RDS: cleanup/fix rds_rdma_unuse
authorAndy Grover <andy.grover@oracle.com>
Tue, 12 Jan 2010 20:37:17 +0000 (12:37 -0800)
committerAndy Grover <andy.grover@oracle.com>
Thu, 9 Sep 2010 01:11:35 +0000 (18:11 -0700)
First, it looks to me like the atomic_inc is wrong.
We should be decrementing refcount only once here, no? It's
already being done by the mr_put() at the end.

Second, simplify the logic a bit by bailing early (with a warning)
if !mr.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
net/rds/rdma.c

index 24274bb..5011efa 100644 (file)
@@ -414,27 +414,30 @@ void rds_rdma_unuse(struct rds_sock *rs, u32 r_key, int force)
 
        spin_lock_irqsave(&rs->rs_rdma_lock, flags);
        mr = rds_mr_tree_walk(&rs->rs_rdma_keys, r_key, NULL);
-       if (mr && (mr->r_use_once || force)) {
+       if (!mr) {
+               printk(KERN_ERR "rds: trying to unuse MR with unknown r_key %u!\n", r_key);
+               spin_unlock_irqrestore(&rs->rs_rdma_lock, flags);
+               return;
+       }
+
+       if (mr->r_use_once || force) {
                rb_erase(&mr->r_rb_node, &rs->rs_rdma_keys);
                RB_CLEAR_NODE(&mr->r_rb_node);
                zot_me = 1;
-       } else if (mr)
-               atomic_inc(&mr->r_refcount);
+       }
        spin_unlock_irqrestore(&rs->rs_rdma_lock, flags);
 
        /* May have to issue a dma_sync on this memory region.
         * Note we could avoid this if the operation was a RDMA READ,
         * but at this point we can't tell. */
-       if (mr) {
-               if (mr->r_trans->sync_mr)
-                       mr->r_trans->sync_mr(mr->r_trans_private, DMA_FROM_DEVICE);
+       if (mr->r_trans->sync_mr)
+               mr->r_trans->sync_mr(mr->r_trans_private, DMA_FROM_DEVICE);
 
-               /* If the MR was marked as invalidate, this will
-                * trigger an async flush. */
-               if (zot_me)
-                       rds_destroy_mr(mr);
-               rds_mr_put(mr);
-       }
+       /* If the MR was marked as invalidate, this will
+        * trigger an async flush. */
+       if (zot_me)
+               rds_destroy_mr(mr);
+       rds_mr_put(mr);
 }
 
 void rds_rdma_free_op(struct rds_rdma_op *ro)