diff options
author | Tina Yang <tina.yang@oracle.com> | 2010-04-01 17:09:00 -0400 |
---|---|---|
committer | Andy Grover <andy.grover@oracle.com> | 2010-09-08 21:07:31 -0400 |
commit | 35b52c70534cb7193b218ec12efe6bc595312097 (patch) | |
tree | 8370e4b3e9d0ed1cc353f8b23531c97f8cc63ddc /net/rds/rdma.c | |
parent | 9e2effba2c16fc3bd47da605116485afe01e0be0 (diff) |
RDS: Fix corrupted rds_mrs
On second look at this bug (OFED #2002), it seems that the
collision is not with the retransmission queue (packet acked
by the peer), but with the local send completion. A theoretical
sequence of events (from time t0 to t3) is thought to be as
follows,
Thread #1
t0:
sock_release
rds_release
rds_send_drop_to /* wait on send completion */
t2:
rds_rdma_drop_keys() /* destroy & free all mrs */
Thread #2
t1:
rds_ib_send_cq_comp_handler
rds_ib_send_unmap_rm
rds_message_unmapped /* wake up #1 @ t0 */
t3:
rds_message_put
rds_message_purge
rds_mr_put /* memory corruption detected */
The problem with the rds_rdma_drop_keys() is it could
remove a mr's refcount more than its due (i.e. repeatedly
as long as it still remains in the tree (mr->r_refcount > 0)).
Theoretically it should remove only one reference - reference
by the tree.
/* Release any MRs associated with this socket */
while ((node = rb_first(&rs->rs_rdma_keys))) {
mr = container_of(node, struct rds_mr, r_rb_node);
if (mr->r_trans == rs->rs_transport)
mr->r_invalidate = 0;
rds_mr_put(mr);
}
I think the correct way of doing it is to remove the mr from
the tree and rds_destroy_mr it first, then a rds_mr_put()
to decrement its reference count by one. Whichever thread
holds the last reference will free the mr via rds_mr_put().
Signed-off-by: Tina Yang <tina.yang@oracle.com>
Signed-off-by: Andy Grover <andy.grover@oracle.com>
Diffstat (limited to 'net/rds/rdma.c')
-rw-r--r-- | net/rds/rdma.c | 8 |
1 files changed, 8 insertions, 0 deletions
diff --git a/net/rds/rdma.c b/net/rds/rdma.c index 3b442d4d64cf..463b458ff27e 100644 --- a/net/rds/rdma.c +++ b/net/rds/rdma.c | |||
@@ -130,14 +130,22 @@ void rds_rdma_drop_keys(struct rds_sock *rs) | |||
130 | { | 130 | { |
131 | struct rds_mr *mr; | 131 | struct rds_mr *mr; |
132 | struct rb_node *node; | 132 | struct rb_node *node; |
133 | unsigned long flags; | ||
133 | 134 | ||
134 | /* Release any MRs associated with this socket */ | 135 | /* Release any MRs associated with this socket */ |
136 | spin_lock_irqsave(&rs->rs_rdma_lock, flags); | ||
135 | while ((node = rb_first(&rs->rs_rdma_keys))) { | 137 | while ((node = rb_first(&rs->rs_rdma_keys))) { |
136 | mr = container_of(node, struct rds_mr, r_rb_node); | 138 | mr = container_of(node, struct rds_mr, r_rb_node); |
137 | if (mr->r_trans == rs->rs_transport) | 139 | if (mr->r_trans == rs->rs_transport) |
138 | mr->r_invalidate = 0; | 140 | mr->r_invalidate = 0; |
141 | rb_erase(&mr->r_rb_node, &rs->rs_rdma_keys); | ||
142 | RB_CLEAR_NODE(&mr->r_rb_node); | ||
143 | spin_unlock_irqrestore(&rs->rs_rdma_lock, flags); | ||
144 | rds_destroy_mr(mr); | ||
139 | rds_mr_put(mr); | 145 | rds_mr_put(mr); |
146 | spin_lock_irqsave(&rs->rs_rdma_lock, flags); | ||
140 | } | 147 | } |
148 | spin_unlock_irqrestore(&rs->rs_rdma_lock, flags); | ||
141 | 149 | ||
142 | if (rs->rs_transport && rs->rs_transport->flush_mrs) | 150 | if (rs->rs_transport && rs->rs_transport->flush_mrs) |
143 | rs->rs_transport->flush_mrs(); | 151 | rs->rs_transport->flush_mrs(); |