Commit 7414dde0 authored by Sagi Grimberg's avatar Sagi Grimberg Committed by Roland Dreier

IB/iser: Fix race between iser connection teardown and scsi TMFs

In certain scenarios (target kill with live IO) scsi TMFs may race
with iser RDMA teardown, which might cause NULL dereference on iser IB
device handle (which might have been freed). In this case we take a
conditional lock for TMFs and check the connection state (avoid
introducing lock contention in the IO path). This is indeed best
effort approach, but sufficient to survive multi targets sudden death
while heavy IO is inflight.

While we are on it, add a nice kernel-doc style documentation.
Reported-by: default avatarAriel Nahum <arieln@mellanox.com>
Signed-off-by: default avatarSagi Grimberg <sagig@mellanox.com>
Signed-off-by: default avatarOr Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: default avatarRoland Dreier <roland@purestorage.com>
parent 3f562a0b
...@@ -164,18 +164,42 @@ iscsi_iser_pdu_alloc(struct iscsi_task *task, uint8_t opcode) ...@@ -164,18 +164,42 @@ iscsi_iser_pdu_alloc(struct iscsi_task *task, uint8_t opcode)
return 0; return 0;
} }
int iser_initialize_task_headers(struct iscsi_task *task, /**
* iser_initialize_task_headers() - Initialize task headers
* @task: iscsi task
* @tx_desc: iser tx descriptor
*
* Notes:
* This routine may race with iser teardown flow for scsi
* error handling TMFs. So for TMF we should acquire the
* state mutex to avoid dereferencing the IB device which
* may have already been terminated.
*/
int
iser_initialize_task_headers(struct iscsi_task *task,
struct iser_tx_desc *tx_desc) struct iser_tx_desc *tx_desc)
{ {
struct iser_conn *iser_conn = task->conn->dd_data; struct iser_conn *iser_conn = task->conn->dd_data;
struct iser_device *device = iser_conn->ib_conn.device; struct iser_device *device = iser_conn->ib_conn.device;
struct iscsi_iser_task *iser_task = task->dd_data; struct iscsi_iser_task *iser_task = task->dd_data;
u64 dma_addr; u64 dma_addr;
const bool mgmt_task = !task->sc && !in_interrupt();
int ret = 0;
if (unlikely(mgmt_task))
mutex_lock(&iser_conn->state_mutex);
if (unlikely(iser_conn->state != ISER_CONN_UP)) {
ret = -ENODEV;
goto out;
}
dma_addr = ib_dma_map_single(device->ib_device, (void *)tx_desc, dma_addr = ib_dma_map_single(device->ib_device, (void *)tx_desc,
ISER_HEADERS_LEN, DMA_TO_DEVICE); ISER_HEADERS_LEN, DMA_TO_DEVICE);
if (ib_dma_mapping_error(device->ib_device, dma_addr)) if (ib_dma_mapping_error(device->ib_device, dma_addr)) {
return -ENOMEM; ret = -ENOMEM;
goto out;
}
tx_desc->dma_addr = dma_addr; tx_desc->dma_addr = dma_addr;
tx_desc->tx_sg[0].addr = tx_desc->dma_addr; tx_desc->tx_sg[0].addr = tx_desc->dma_addr;
...@@ -183,7 +207,11 @@ int iser_initialize_task_headers(struct iscsi_task *task, ...@@ -183,7 +207,11 @@ int iser_initialize_task_headers(struct iscsi_task *task,
tx_desc->tx_sg[0].lkey = device->mr->lkey; tx_desc->tx_sg[0].lkey = device->mr->lkey;
iser_task->iser_conn = iser_conn; iser_task->iser_conn = iser_conn;
return 0; out:
if (unlikely(mgmt_task))
mutex_unlock(&iser_conn->state_mutex);
return ret;
} }
/** /**
...@@ -199,9 +227,14 @@ static int ...@@ -199,9 +227,14 @@ static int
iscsi_iser_task_init(struct iscsi_task *task) iscsi_iser_task_init(struct iscsi_task *task)
{ {
struct iscsi_iser_task *iser_task = task->dd_data; struct iscsi_iser_task *iser_task = task->dd_data;
int ret;
if (iser_initialize_task_headers(task, &iser_task->desc)) ret = iser_initialize_task_headers(task, &iser_task->desc);
return -ENOMEM; if (ret) {
iser_err("Failed to init task %p, err = %d\n",
iser_task, ret);
return ret;
}
/* mgmt task */ /* mgmt task */
if (!task->sc) if (!task->sc)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment