[PATCH 3/6] nvme: tcp: unquiesce queues after ctrl state is changed

Sagi Grimberg sagi at grimberg.me
Tue Jul 11 00:41:46 PDT 2023


> Unquiesce queues until controller state is changed successfully.
> 
> Prepare for moving start freeze into reconnect work for preventing new
> request from entering queue in LIVE state.
> 
> Before removing namespace, unquiescing queues is done unconditionally, so
> it is safe to do this way in case of state change failure.
> 
> Signed-off-by: Ming Lei <ming.lei at redhat.com>
> ---
>   drivers/nvme/host/tcp.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index e414cfb3433f..e68bf21af348 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2109,10 +2109,7 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work)
>   	nvme_stop_keep_alive(ctrl);
>   	flush_work(&ctrl->async_event_work);
>   	nvme_tcp_teardown_io_queues(ctrl);
> -	/* unquiesce to fail fast pending requests */
> -	nvme_unquiesce_io_queues(ctrl);
>   	nvme_tcp_teardown_admin_queue(ctrl);
> -	nvme_unquiesce_admin_queue(ctrl);
>   	nvme_auth_stop(ctrl);

This is potentially a problem. It is possible that auth work is
running in the bg here, and without unquiescing the admin queue
the the auth_stop may take unnecessarily long until its I/O times
out.

I think it is fine to keep it as is...

>   
>   	if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING)) {
> @@ -2122,6 +2119,10 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work)
>   		return;
>   	}
>   
> +	/* unquiesce to fail fast pending requests */
> +	nvme_unquiesce_io_queues(ctrl);
> +	nvme_unquiesce_admin_queue(ctrl);
> +
>   	nvme_tcp_reconnect_or_remove(ctrl);
>   }
>   



More information about the Linux-nvme mailing list