[PATCH 0/4] nvme: start keep-alive after admin queue setup

[Hannes Reinecke]

On 10/20/23 17:30, Keith Busch wrote:
> On Fri, Oct 20, 2023 at 04:25:56PM +0200, Hannes Reinecke wrote:
>> Hi all,
>>
>> Setting up I/O queues might take quite some time on larger and/or
>> busy setups, so KATO might expire on the admin queue before all
>> I/O queues can be setup.
>> This patchset fixes this issue by moving the calls to starting
>> and stopping KATO into the quiesce/unquiesce functions.
>> That requires some refactoring/alignment with some transport
>> drivers which do not always quiesce and unquiesce the admin
>> queue.
>>
>> As usual, comments and reviews are welcome.
> 
> The pci transport doesn't care about keep-alive, so I don't think we
> should force everyone to have to consider it.
> 
> Wouldn't you want to start the keep-alive on the connecting state
> transition? Something like this:
> 
> ---
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 62612f87aafa2..a44b1206b20ad 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -583,6 +583,8 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
>          } else if (ctrl->state == NVME_CTRL_CONNECTING &&
>                  old_state == NVME_CTRL_RESETTING) {
>                  nvme_start_failfast_work(ctrl);
> +               if (ctrl->ops->flags & NVME_F_FABRICS) {
> +                       nvme_start_keep_alive(ctrl);
>          }
>          return changed;
>   }
> --
> 
But we start keep alive unconditionally nowadays; each and every
host driver (including PCI) is calling into nvme_start_ctrl(),
which calls nvme_start_keep_alive() without any checks.

Be it as it may, tying it into the state transition won't work
as the 'CONNECTING' state is entered _before_ any queues are setup,
so starting keep-alive here would be too early.

Cheers,

Hannes