[PATCH 1/3] Target/iser: Get isert_conn reference once got to connected_handler

[Sagi Grimberg]

In case the connection didn't reach connected state, disconnected
handler will never be invoked thus the second kref_put on
isert_conn will be missing.

Signed-off-by: Sagi Grimberg <***@mellanox.com>
---
drivers/infiniband/ulp/isert/ib_isert.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 52b13ff..59cb284 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -586,7 +586,6 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
init_completion(&isert_conn->conn_wait);
init_completion(&isert_conn->conn_wait_comp_err);
kref_init(&isert_conn->conn_kref);
- kref_get(&isert_conn->conn_kref);
mutex_init(&isert_conn->conn_mutex);
spin_lock_init(&isert_conn->conn_lock);
INIT_LIST_HEAD(&isert_conn->conn_fr_pool);
@@ -748,7 +747,9 @@ isert_connect_release(struct isert_conn *isert_conn)
static void
isert_connected_handler(struct rdma_cm_id *cma_id)
{
- return;
+ struct isert_conn *isert_conn = cma_id->context;
+
+ kref_get(&isert_conn->conn_kref);
}

static void

[Sagi Grimberg]

rdma_disconnect may be called in 2 code flows:
- isert_wait_conn: disconnect initiated be the target
- disconnected_handler: disconnect invoked by the initiator

In case isert_conn->disconnect is true then rdma_disconnect
was called in disconnected handler, no need to call it again
from isert_wait_conn.

Signed-off-by: Sagi Grimberg <***@mellanox.com>
---
drivers/infiniband/ulp/isert/ib_isert.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 03a5e43..7dcdff3 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -3214,7 +3214,7 @@ static void isert_wait_conn(struct iscsi_conn *conn)
pr_debug("isert_wait_conn: Starting \n");

mutex_lock(&isert_conn->conn_mutex);
- if (isert_conn->conn_cm_id) {
+ if (isert_conn->conn_cm_id && !isert_conn->disconnect) {
pr_debug("Calling rdma_disconnect from isert_wait_conn\n");
rdma_disconnect(isert_conn->conn_cm_id);
}

[Sagi Grimberg]

disconnected_handler is invoked on several CM events (such
as DISCONNECTED, DEVICE_REMOVAL, TIMEWAIT_EXIT...). Since
multiple events can occur while before isert_free_conn is
invoked, we might put all isert_conn references and free
the connection too early.

Signed-off-by: Sagi Grimberg <***@mellanox.com>
---
drivers/infiniband/ulp/isert/ib_isert.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 59cb284..03a5e43 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -801,7 +801,6 @@ isert_disconnect_work(struct work_struct *work)

wake_up:
complete(&isert_conn->conn_wait);
- isert_put_conn(isert_conn);
}

static void
@@ -3234,6 +3233,7 @@ static void isert_wait_conn(struct iscsi_conn *conn)
wait_for_completion(&isert_conn->conn_wait_comp_err);

wait_for_completion(&isert_conn->conn_wait);
+ isert_put_conn(isert_conn);
}

static void isert_free_conn(struct iscsi_conn *conn)

[Christoph Hellwig]

Nic,

did you manage to take a look at these patches?

Patch #1: Fix a possible hang when connection establishment breaks
before entering connected state
Patch #2: Fix possible early free of isert_conn
Patch #3: Nit fix of isert work with rdma_cm
Target/iser: Get isert_conn reference once got to connected_handler
Target/iser: Don't put isert_conn inside disconnected handler
Target/iser: Avoid calling rdma_disconnect twice
drivers/infiniband/ulp/isert/ib_isert.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

---end quoted text---

[Nicholas A. Bellinger]

Post by Christoph Hellwig
Nic,
did you manage to take a look at these patches?

Yep, applied to for-next.

Also Sagi, I assume that the first two should be CC'ed to v3.10.y,
yes..?

--nab

Post by Christoph Hellwig

Patch #1: Fix a possible hang when connection establishment breaks
before entering connected state
Patch #2: Fix possible early free of isert_conn
Patch #3: Nit fix of isert work with rdma_cm
Target/iser: Get isert_conn reference once got to connected_handler
Target/iser: Don't put isert_conn inside disconnected handler
Target/iser: Avoid calling rdma_disconnect twice
drivers/infiniband/ulp/isert/ib_isert.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Sagi Grimberg]

Post by Nicholas A. Bellinger

Post by Christoph Hellwig
Nic,
did you manage to take a look at these patches?

Yep, applied to for-next.

Thanks Nic.

Post by Nicholas A. Bellinger
Also Sagi, I assume that the first two should be CC'ed to v3.10.y,
yes..?

I need to test that. do they apply cleanly?

Sagi.