Re: [PATCH] nvme-tcp: proper handling of tcp socket closing flows
From: Sagi Grimberg <sagi@grimberg.me>
Date: 2021-01-28 07:55:22
From: Elad Grupi <redacted> avoid calling nvmet_tcp_release_queue_work if tcp socket was closed before setting the sk callbacks.
But we need to call the release_queue work, not sure I understand how this works.
prevent io_work from enqueuing while closing the tcp queue to avoid race with nvmet_tcp_release_queue_work.
io_work is going to run with release_queue_work, because it is also coming from the backend completion. The right way to solve this is to correctly fence them. Can you describe the exact race you are referring to?
quoted hunk ↗ jump to hunk
Signed-off-by: Elad Grupi <redacted> --- drivers/nvme/target/tcp.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c index d535080b781f..937f2a746d8b 100644 --- a/drivers/nvme/target/tcp.c +++ b/drivers/nvme/target/tcp.c@@ -647,7 +647,7 @@ static int nvmet_tcp_try_send_one(struct nvmet_tcp_queue *queue, struct nvmet_tcp_cmd *cmd = queue->snd_cmd; int ret = 0; - if (!cmd || queue->state == NVMET_TCP_Q_DISCONNECTING) { + if (!cmd) { cmd = nvmet_tcp_fetch_cmd(queue); if (unlikely(!cmd)) return 0;@@ -1196,7 +1196,7 @@ static void nvmet_tcp_io_work(struct work_struct *w) /* * We exahusted our budget, requeue our selves */ - if (pending) + if (pending && queue->state != NVMET_TCP_Q_DISCONNECTING) queue_work_on(queue->cpu, nvmet_tcp_wq, &queue->io_work); }@@ -1453,9 +1453,27 @@ static int nvmet_tcp_set_queue_sock(struct nvmet_tcp_queue *queue) sock->sk->sk_state_change = nvmet_tcp_state_change; queue->write_space = sock->sk->sk_write_space; sock->sk->sk_write_space = nvmet_tcp_write_space; + + switch (sk->sk_state) { + case TCP_FIN_WAIT1: + case TCP_CLOSE_WAIT: + case TCP_CLOSE: + /* FALLTHRU */ + sock->sk->sk_data_ready = queue->data_ready; + sock->sk->sk_state_change = queue->state_change; + sock->sk->sk_write_space = queue->write_space; + sk->sk_user_data = NULL; + queue->state = NVMET_TCP_Q_DISCONNECTING; + ret = -ENOTCONN; + break; + default: + queue_work_on(queue->cpu, nvmet_tcp_wq, &queue->io_work); + ret = 0; + } + write_unlock_bh(&sock->sk->sk_callback_lock); - return 0; + return ret; } static int nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port,@@ -1506,8 +1524,6 @@ static int nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port, if (ret) goto out_destroy_sq; - queue_work_on(queue->cpu, nvmet_tcp_wq, &queue->io_work); - return 0; out_destroy_sq: mutex_lock(&nvmet_tcp_queue_mutex);
_______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme