Skip to content

Commit

Permalink
Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
Browse files Browse the repository at this point in the history
RDMA CM and iSCSI target flows are asynchronous and completely
uncorrelated. Relying on the fact that iscsi_accept_np will be called
after CM connection request event and will wait for it is a mistake.

When attempting to login to a few targets this flow is racy and
unpredictable, but for parallel login to dozens of targets will
race and hang every time.

The correct synchronizing mechanism in this case is pending on
a semaphore rather than a wait_for_event. We keep the pending
interruptible for iscsi_np cleanup stage.

(Squash patch to remove dead code into parent - nab)

Reported-by: Slava Shwartsman <valyushash@gmail.com>
Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
  • Loading branch information
Sagi Grimberg authored and Nicholas Bellinger committed May 16, 2014
1 parent 9fe63c8 commit 531b7bf
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 20 deletions.
25 changes: 6 additions & 19 deletions drivers/infiniband/ulp/isert/ib_isert.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <target/target_core_base.h>
#include <target/target_core_fabric.h>
#include <target/iscsi/iscsi_transport.h>
#include <linux/semaphore.h>

#include "isert_proto.h"
#include "ib_isert.h"
Expand Down Expand Up @@ -666,8 +667,8 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
list_add_tail(&isert_conn->conn_accept_node, &isert_np->np_accept_list);
mutex_unlock(&isert_np->np_accept_mutex);

pr_debug("isert_connect_request() waking up np_accept_wq: %p\n", np);
wake_up(&isert_np->np_accept_wq);
pr_debug("isert_connect_request() up np_sem np: %p\n", np);
up(&isert_np->np_sem);
return 0;

out_conn_dev:
Expand Down Expand Up @@ -2999,7 +3000,7 @@ isert_setup_np(struct iscsi_np *np,
pr_err("Unable to allocate struct isert_np\n");
return -ENOMEM;
}
init_waitqueue_head(&isert_np->np_accept_wq);
sema_init(&isert_np->np_sem, 0);
mutex_init(&isert_np->np_accept_mutex);
INIT_LIST_HEAD(&isert_np->np_accept_list);
init_completion(&isert_np->np_login_comp);
Expand Down Expand Up @@ -3047,18 +3048,6 @@ isert_setup_np(struct iscsi_np *np,
return ret;
}

static int
isert_check_accept_queue(struct isert_np *isert_np)
{
int empty;

mutex_lock(&isert_np->np_accept_mutex);
empty = list_empty(&isert_np->np_accept_list);
mutex_unlock(&isert_np->np_accept_mutex);

return empty;
}

static int
isert_rdma_accept(struct isert_conn *isert_conn)
{
Expand Down Expand Up @@ -3151,16 +3140,14 @@ isert_accept_np(struct iscsi_np *np, struct iscsi_conn *conn)
int max_accept = 0, ret;

accept_wait:
ret = wait_event_interruptible(isert_np->np_accept_wq,
!isert_check_accept_queue(isert_np) ||
np->np_thread_state == ISCSI_NP_THREAD_RESET);
ret = down_interruptible(&isert_np->np_sem);
if (max_accept > 5)
return -ENODEV;

spin_lock_bh(&np->np_thread_lock);
if (np->np_thread_state == ISCSI_NP_THREAD_RESET) {
spin_unlock_bh(&np->np_thread_lock);
pr_err("ISCSI_NP_THREAD_RESET for isert_accept_np\n");
pr_debug("ISCSI_NP_THREAD_RESET for isert_accept_np\n");
return -ENODEV;
}
spin_unlock_bh(&np->np_thread_lock);
Expand Down
2 changes: 1 addition & 1 deletion drivers/infiniband/ulp/isert/ib_isert.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ struct isert_device {
};

struct isert_np {
wait_queue_head_t np_accept_wq;
struct semaphore np_sem;
struct rdma_cm_id *np_cm_id;
struct mutex np_accept_mutex;
struct list_head np_accept_list;
Expand Down

0 comments on commit 531b7bf

Please sign in to comment.