Re: [PATCH 09/15] IB/srpt: Fix srpt_close_session()

[Sagi Grimberg]

Avoid that srpt_close_session() waits if it doesn't have to wait.


Can you explain when it doesn't have to wait? is it possible that
srpt_release_channel_work() was already triggered? isn't that a problem?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/15] IB/srpt: Fix srpt_close_session()

[Bart Van Assche]

Avoid that srpt_close_session() waits if it doesn't have to wait.
Additionally, increase the time during which srpt_close_session()
waits until closing a session has finished. This makes it easier
to detect session shutdown bugs.

Signed-off-by: Bart Van Assche 
Cc: Christoph Hellwig 
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c 
b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 0ff4ed6..1857d17 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1981,8 +1981,8 @@ static void srpt_release_channel_work(struct work_struct 
*w)
struct se_session *se_sess;
 
ch = container_of(w, struct srpt_rdma_ch, release_work);
-   pr_debug("ch = %p; ch->sess = %p; release_done = %p\n", ch, ch->sess,
-ch->release_done);
+   pr_debug("%s: %s-%d; release_done = %p\n", __func__, ch->sess_name,
+ch->qp->qp_num, ch->release_done);
 
sdev = ch->sport->sdev;
BUG_ON(!sdev);
@@ -2006,11 +2006,10 @@ static void srpt_release_channel_work(struct 
work_struct *w)
 ch->rsp_size, DMA_TO_DEVICE);
 
spin_lock_irq(>spinlock);
-   list_del(>list);
-   spin_unlock_irq(>spinlock);
-
+   list_del_init(>list);
if (ch->release_done)
complete(ch->release_done);
+   spin_unlock_irq(>spinlock);
 
wake_up(>ch_releaseQ);
 
@@ -3033,24 +3032,26 @@ static void srpt_release_cmd(struct se_cmd *se_cmd)
 static void srpt_close_session(struct se_session *se_sess)
 {
DECLARE_COMPLETION_ONSTACK(release_done);
-   struct srpt_rdma_ch *ch;
-   struct srpt_device *sdev;
-   unsigned long res;
-
-   ch = se_sess->fabric_sess_ptr;
-   WARN_ON(ch->sess != se_sess);
+   struct srpt_rdma_ch *ch = se_sess->fabric_sess_ptr;
+   struct srpt_device *sdev = ch->sport->sdev;
+   bool wait;
 
-   pr_debug("ch %p state %d\n", ch, ch->state);
+   pr_debug("ch %s-%d state %d\n", ch->sess_name, ch->qp->qp_num,
+ch->state);
 
-   sdev = ch->sport->sdev;
spin_lock_irq(>spinlock);
BUG_ON(ch->release_done);
ch->release_done = _done;
+   wait = !list_empty(>list);
__srpt_close_ch(ch);
spin_unlock_irq(>spinlock);
 
-   res = wait_for_completion_timeout(_done, 60 * HZ);
-   WARN_ON(res == 0);
+   if (!wait)
+   return;
+
+   while (wait_for_completion_timeout(_done, 180 * HZ) == 0)
+   pr_info("%s(%s-%d state %d): still waiting ...\n", __func__,
+   ch->sess_name, ch->qp->qp_num, ch->state);
 }
 
 /**
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/15] IB/srpt: Fix srpt_close_session()

[Christoph Hellwig]

On Tue, Jan 05, 2016 at 03:24:49PM +0100, Bart Van Assche wrote:
> Avoid that srpt_close_session() waits if it doesn't have to wait.
> Additionally, increase the time during which srpt_close_session()
> waits until closing a session has finished. This makes it easier
> to detect session shutdown bugs.

Looks good,

Reviewed-by: Christoph Hellwig 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html