[PATCH 2/3] nvme: sanitize KATO setting

[Max Gurtovoy]

On 3/2/2021 11:26 AM, Hannes Reinecke wrote:
> According to the NVMe base spec the KATO commands should be sent
> at half of the KATO interval, to properly account for round-trip
> times.
> As we now will only ever send one KATO command per connection we
> can easily use the recommended values.
> This also fixes a potential issue where the request timeout for
> the KATO command does not match the value in the connect command,
> which might be causing spurious connection drops from the target.
>
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> ---
>   drivers/nvme/host/core.c    | 9 ++++++---
>   drivers/nvme/host/fabrics.c | 2 +-
>   drivers/nvme/host/nvme.h    | 9 ++++++++-
>   3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index f890b310499e..6d096d41a82f 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1223,7 +1223,8 @@ static void nvme_keep_alive_end_io(struct request *rq, blk_status_t status)
>   		startka = true;
>   	spin_unlock_irqrestore(&ctrl->lock, flags);
>   	if (startka)
> -		queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);
> +		queue_delayed_work(nvme_wq, &ctrl->ka_work,
> +				   NVME_KATO_DELAY(ctrl->kato) * HZ);
>   }
>   
>   static int nvme_keep_alive(struct nvme_ctrl *ctrl)
> @@ -1258,7 +1259,8 @@ static void nvme_keep_alive_work(struct work_struct *work)
>   		dev_dbg(ctrl->device,
>   			"reschedule traffic based keep-alive timer\n");
>   		ctrl->comp_seen = false;
> -		queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);
> +		queue_delayed_work(nvme_wq, &ctrl->ka_work,
> +				   NVME_KATO_DELAY(ctrl->kato) * HZ);
>   		return;
>   	}
>   
> @@ -1275,7 +1277,8 @@ static void nvme_start_keep_alive(struct nvme_ctrl *ctrl)
>   	if (unlikely(ctrl->kato == 0))
>   		return;
>   
> -	queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);
> +	queue_delayed_work(nvme_wq, &ctrl->ka_work,
> +			   NVME_KATO_DELAY(ctrl->kato) * HZ);
>   }
>   
>   void nvme_stop_keep_alive(struct nvme_ctrl *ctrl)
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 5dfd806fc2d2..dba32e39afbf 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -382,7 +382,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
>   	 * and add a grace period for controller kato enforcement
>   	 */
>   	cmd.connect.kato = ctrl->kato ?
> -		cpu_to_le32((ctrl->kato + NVME_KATO_GRACE) * 1000) : 0;
> +		cpu_to_le32(ctrl->kato * 1000) : 0;
>   
>   	if (ctrl->opts->disable_sqflow)
>   		cmd.connect.cattr |= NVME_CONNECT_DISABLE_SQFLOW;
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 23711f6b7d13..912830389997 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -27,7 +27,14 @@ extern unsigned int admin_timeout;
>   #define NVME_ADMIN_TIMEOUT	(admin_timeout * HZ)
>   
>   #define NVME_DEFAULT_KATO	5
> -#define NVME_KATO_GRACE		10
> +
> +/*
> + * The recommended frequency for KATO commands
> + * according to NVMe 1.4 section 7.12.1:
> + * The host should send Keep Alive commands at half of the
> + * Keep Alive Timeout accounting for transport roundtrip times [..].
> + */
> +#define NVME_KATO_DELAY(k)	((k) >> 1)

what will happen in case k == 1 ?


>   
>   #ifdef CONFIG_ARCH_NO_SG_CHAIN
>   #define  NVME_INLINE_SG_CNT  0