RE: [PATCH] nvme: update keep alive interval when kato is modified
From: sasaki tatsuya <hidden>
Date: 2021-08-19 10:15:52
Also in:
lkml
This is too specific. I'd move it to nvme_user_cmd_post similar to how we handle passthru commands (with start/end that does stuff based on the effects)
Thank you for your comment. I will recreate this patch which adds nvme_user_cmd_post for update keep alive timer and move update routine to core.c. -----Original Message----- From: Sagi Grimberg <sagi@grimberg.me> Sent: Saturday, August 7, 2021 5:07 AM To: sasaki tatsuya(佐々木 達哉 KIC ○S技C□SS開○SS一) <redacted>; kernel test robot <redacted>; kbusch@kernel.org; axboe@fb.com; hch@lst.de; linux-nvme@lists.infradead.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] nvme: update keep alive interval when kato is modified On 8/2/21 7:12 PM, sasaki tatsuya wrote:
quoted hunk ↗ jump to hunk
Currently the connection between host and NVMe-oF target gets disconnected by keep-alive timeout when a user connects to a target with a relatively large kato value and then sets the smaller kato with a set features command (e.g. connects with 60 seconds kato value and then sets 10 seconds kato value). The cause is that keep alive command interval on the host, which is defined as unsigned int kato in nvme_ctrl structure, does not follow the kato value changes. This patch updates the keep alive interval in the following steps when the kato is modified by a set features command: stops the keep alive work queue, then sets the kato as new timer value and re-start the queue. Signed-off-by: Tatsuya Sasaki <redacted> --- drivers/nvme/host/core.c | 3 ++- drivers/nvme/host/ioctl.c | 17 +++++++++++++++++ drivers/nvme/host/nvme.h | 1 + 3 files changed, 20 insertions(+), 1 deletion(-)diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index dfd9dec0c1f6..89c52da15618 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c@@ -1255,13 +1255,14 @@ static void nvme_keep_alive_work(struct work_struct *work) blk_execute_rq_nowait(NULL, rq, 0, nvme_keep_alive_end_io); } -static void nvme_start_keep_alive(struct nvme_ctrl *ctrl) +void nvme_start_keep_alive(struct nvme_ctrl *ctrl) { if (unlikely(ctrl->kato == 0)) return; nvme_queue_keep_alive_work(ctrl); } +EXPORT_SYMBOL_GPL(nvme_start_keep_alive); void nvme_stop_keep_alive(struct nvme_ctrl *ctrl) {diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 305ddd415e45..0066728e77b2 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c@@ -231,6 +231,23 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns, return -EFAULT; } + /* + * Keep alive commands interval on the host should be updated + * when KATO is modified by Set Features commands. + */
This is too specific. I'd move it to nvme_user_cmd_post similar to how we handle passthru commands (with start/end that does stuff based on the effects)
+ if (!status && cmd.opcode == nvme_admin_set_features &&
+ (cmd.cdw10 & 0xFF) == NVME_FEAT_KATO) {
+ /* ms -> s */
+ unsigned int new_kato = DIV_ROUND_UP(cmd.cdw11, 1000);
+The section below needs to move to a core rountine: nvme_update_keep_alive()
+ dev_info(ctrl->device, + "keep alive commands interval on the host is updated from %u milliseconds to %u milliseconds\n", + ctrl->kato * 1000 / 2, new_kato * 1000 / 2); + nvme_stop_keep_alive(ctrl); + ctrl->kato = new_kato; + nvme_start_keep_alive(ctrl); + } + return status; }
_______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme