Skip to content

Commit

Permalink
nvme-tcp: fix premature queue removal and I/O failover
Browse files Browse the repository at this point in the history
This patch addresses a data corruption issue observed in nvme-tcp during
testing.

In an NVMe native multipath setup, when an I/O timeout occurs, all
inflight I/Os are canceled almost immediately after the kernel socket is
shut down. These canceled I/Os are reported as host path errors,
triggering a failover that succeeds on a different path.

However, at this point, the original I/O may still be outstanding in the
host's network transmission path (e.g., the NIC’s TX queue). From the
user-space app's perspective, the buffer associated with the I/O is
considered completed since they're acked on the different path and may
be reused for new I/O requests.

Because nvme-tcp enables zero-copy by default in the transmission path,
this can lead to corrupted data being sent to the original target,
ultimately causing data corruption.

We can reproduce this data corruption by injecting delay on one path and
triggering i/o timeout.

To prevent this issue, this change ensures that all inflight
transmissions are fully completed from host's perspective before
returning from queue stop. To handle concurrent I/O timeout from multiple
namespaces under the same controller, always wait in queue stop
regardless of queue's state.

This aligns with the behavior of queue stopping in other NVMe fabric
transports.

Fixes: 3f2304f ("nvme-tcp: add NVMe over TCP host driver")
Signed-off-by: Michael Liang <mliang@purestorage.com>
Reviewed-by: Mohamed Khalfella <mkhalfella@purestorage.com>
Reviewed-by: Randy Jennings <randyj@purestorage.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Christoph Hellwig <hch@lst.de>
  • Loading branch information
Michael Liang authored and Christoph Hellwig committed Apr 30, 2025
1 parent ab35ad9 commit 77e40bb
Showing 1 changed file with 29 additions and 2 deletions.
31 changes: 29 additions & 2 deletions drivers/nvme/host/tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1946,7 +1946,7 @@ static void __nvme_tcp_stop_queue(struct nvme_tcp_queue *queue)
cancel_work_sync(&queue->io_work);
}

static void nvme_tcp_stop_queue(struct nvme_ctrl *nctrl, int qid)
static void nvme_tcp_stop_queue_nowait(struct nvme_ctrl *nctrl, int qid)
{
struct nvme_tcp_ctrl *ctrl = to_tcp_ctrl(nctrl);
struct nvme_tcp_queue *queue = &ctrl->queues[qid];
Expand All @@ -1965,6 +1965,31 @@ static void nvme_tcp_stop_queue(struct nvme_ctrl *nctrl, int qid)
mutex_unlock(&queue->queue_lock);
}

static void nvme_tcp_wait_queue(struct nvme_ctrl *nctrl, int qid)
{
struct nvme_tcp_ctrl *ctrl = to_tcp_ctrl(nctrl);
struct nvme_tcp_queue *queue = &ctrl->queues[qid];
int timeout = 100;

while (timeout > 0) {
if (!test_bit(NVME_TCP_Q_ALLOCATED, &queue->flags) ||
!sk_wmem_alloc_get(queue->sock->sk))
return;
msleep(2);
timeout -= 2;
}
dev_warn(nctrl->device,
"qid %d: timeout draining sock wmem allocation expired\n",
qid);
}

static void nvme_tcp_stop_queue(struct nvme_ctrl *nctrl, int qid)
{
nvme_tcp_stop_queue_nowait(nctrl, qid);
nvme_tcp_wait_queue(nctrl, qid);
}


static void nvme_tcp_setup_sock_ops(struct nvme_tcp_queue *queue)
{
write_lock_bh(&queue->sock->sk->sk_callback_lock);
Expand Down Expand Up @@ -2032,7 +2057,9 @@ static void nvme_tcp_stop_io_queues(struct nvme_ctrl *ctrl)
int i;

for (i = 1; i < ctrl->queue_count; i++)
nvme_tcp_stop_queue(ctrl, i);
nvme_tcp_stop_queue_nowait(ctrl, i);
for (i = 1; i < ctrl->queue_count; i++)
nvme_tcp_wait_queue(ctrl, i);
}

static int nvme_tcp_start_io_queues(struct nvme_ctrl *ctrl,
Expand Down

0 comments on commit 77e40bb

Please sign in to comment.