From 70110ec078ce21fc695742812968e354155f72e2 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 3 Aug 2009 16:00:36 +0000 Subject: [PATCH] Fixed race conditions on RDMA channel manipulations found via source reading: - Added a second argument to srpt_find_channel() that specified whether or not the channel should be removed from the channel list if found. - Moved list_del() statement from the body of the srpt_release_channel() function to its callers. git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@1005 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 47 +++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index fe816002a..93ca49e8f 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -1337,7 +1337,7 @@ out: * * Return NULL if no matching RDMA channel has been found. */ -static struct srpt_rdma_ch *srpt_find_channel(struct ib_cm_id *cm_id) +static struct srpt_rdma_ch *srpt_find_channel(struct ib_cm_id *cm_id, bool del) { struct srpt_device *sdev = cm_id->context; struct srpt_rdma_ch *ch; @@ -1345,6 +1345,8 @@ static struct srpt_rdma_ch *srpt_find_channel(struct ib_cm_id *cm_id) spin_lock_irq(&sdev->spinlock); list_for_each_entry(ch, &sdev->rch_list, list) { if (ch->cm_id == cm_id) { + if (del) + list_del(&ch->list); spin_unlock_irq(&sdev->spinlock); return ch; } @@ -1355,19 +1357,17 @@ static struct srpt_rdma_ch *srpt_find_channel(struct ib_cm_id *cm_id) return NULL; } -/** Release all resources associated with the specified RDMA channel. */ +/** + * Release all resources associated with the specified RDMA channel. + * + * Note: the caller must have removed the channel from the channel list + * before calling this function. + */ static void srpt_release_channel(struct srpt_rdma_ch *ch, int destroy_cmid) { TRACE_ENTRY(); - /* - * Remove the channel from the channel list such that - * srpt_find_channel() won't find this channel anymore and hence - * new incoming requests for this channel will be refused. - */ - spin_lock_irq(&ch->sport->sdev->spinlock); - list_del(&ch->list); - spin_unlock_irq(&ch->sport->sdev->spinlock); + WARN_ON(srpt_find_channel(ch->cm_id, false) == ch); if (ch->cm_id && destroy_cmid) { TRACE_DBG("%s: destroy cm_id= %p", __func__, ch->cm_id); @@ -1482,6 +1482,9 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, " cm_id= %p state= %d", ch->sess_name, ch->cm_id, ch->state); + if (ch->state == RDMA_CHANNEL_CONNECTING) + list_del(&ch->list); + spin_unlock_irq(&sdev->spinlock); rsp->rsp_flags = @@ -1563,10 +1566,6 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, goto destroy_ib; } - spin_lock_irq(&sdev->spinlock); - list_add_tail(&ch->list, &sdev->rch_list); - spin_unlock_irq(&sdev->spinlock); - TRACE_DBG("Establish connection sess=%p name=%s cm_id=%p", ch->scst_sess, ch->sess_name, ch->cm_id); @@ -1594,7 +1593,11 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, rep_param->initiator_depth = 4; ret = ib_send_cm_rep(cm_id, rep_param); - if (ret) + if (ret == 0) { + spin_lock_irq(&sdev->spinlock); + list_add_tail(&ch->list, &sdev->rch_list); + spin_unlock_irq(&sdev->spinlock); + } else srpt_release_channel(ch, 0); goto out; @@ -1633,7 +1636,7 @@ static void srpt_find_and_release_channel(struct ib_cm_id *cm_id) { struct srpt_rdma_ch *ch; - ch = srpt_find_channel(cm_id); + ch = srpt_find_channel(cm_id, true); if (ch) srpt_release_channel(ch, 0); } @@ -1649,7 +1652,7 @@ static int srpt_cm_rtu_recv(struct ib_cm_id *cm_id) struct srpt_rdma_ch *ch; int ret; - ch = srpt_find_channel(cm_id); + ch = srpt_find_channel(cm_id, false); if (!ch) return -EINVAL; @@ -1696,7 +1699,7 @@ static int srpt_cm_dreq_recv(struct ib_cm_id *cm_id) { struct srpt_rdma_ch *ch; - ch = srpt_find_channel(cm_id); + ch = srpt_find_channel(cm_id, false); if (!ch) return -EINVAL; @@ -2257,8 +2260,14 @@ static int srpt_release(struct scst_tgt *scst_tgt) srpt_unregister_procfs_entry(scst_tgt->tgtt); - list_for_each_entry_safe(ch, tmp_ch, &sdev->rch_list, list) + spin_lock_irq(&sdev->spinlock); + list_for_each_entry_safe(ch, tmp_ch, &sdev->rch_list, list) { + list_del(&ch->list); + spin_unlock_irq(&sdev->spinlock); srpt_release_channel(ch, 1); + spin_lock_irq(&sdev->spinlock); + } + spin_unlock_irq(&sdev->spinlock); srpt_unregister_mad_agent(sdev);