RE: [PATCH] nvme: update keep alive interval when kato is modified

From: sasaki tatsuya
Date: Mon Aug 02 2021 - 22:13:31 EST


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 <tatsuya6.sasaki@xxxxxxxxxx>
---
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.
+ */
+ 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);
+
+ 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;
}

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 5cd1fa3b8464..d4066b7c5fc8 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -666,6 +666,7 @@ int nvme_get_features(struct nvme_ctrl *dev, unsigned int fid,
unsigned int dword11, void *buffer, size_t buflen,
u32 *result);
int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count);
+void nvme_start_keep_alive(struct nvme_ctrl *ctrl);
void nvme_stop_keep_alive(struct nvme_ctrl *ctrl);
int nvme_reset_ctrl(struct nvme_ctrl *ctrl);
int nvme_reset_ctrl_sync(struct nvme_ctrl *ctrl);
--
2.25.1

-----Original Message-----
From: kernel test robot <lkp@xxxxxxxxx>
Sent: Monday, August 2, 2021 1:32 PM
To: sasaki tatsuya(佐々木 達哉 KIC ○S技C□SS開○SS一) <tatsuya6.sasaki@xxxxxxxxxx>; kbusch@xxxxxxxxxx; axboe@xxxxxx; hch@xxxxxx; sagi@xxxxxxxxxxx; linux-nvme@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Cc: kbuild-all@xxxxxxxxxxxx
Subject: Re: [PATCH] nvme: update keep alive interval when kato is modified

Hi sasaki,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on block/for-next]
[also build test WARNING on hch-configfs/for-next linus/master v5.14-rc3 next-20210730]
[cannot apply to linux-nvme/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/sasaki-tatsuya/nvme-update-keep-alive-interval-when-kato-is-modified/20210802-090235
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: i386-randconfig-s002-20210802 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/eda2903523c28b51997fb071c0ff3653081c8a79
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review sasaki-tatsuya/nvme-update-keep-alive-interval-when-kato-is-modified/20210802-090235
git checkout eda2903523c28b51997fb071c0ff3653081c8a79
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/nvme/host/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>


sparse warnings: (new ones prefixed by >>)
>> drivers/nvme/host/ioctl.c:239:15: sparse: sparse: cast from restricted __le32
>> drivers/nvme/host/ioctl.c:241:41: sparse: sparse: restricted __le32 degrades to integer

vim +239 drivers/nvme/host/ioctl.c

189
190 static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
191 struct nvme_passthru_cmd __user *ucmd)
192 {
193 struct nvme_passthru_cmd cmd;
194 struct nvme_command c;
195 unsigned timeout = 0;
196 u64 result;
197 int status;
198
199 if (!capable(CAP_SYS_ADMIN))
200 return -EACCES;
201 if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
202 return -EFAULT;
203 if (cmd.flags)
204 return -EINVAL;
205 if (!nvme_validate_passthru_nsid(ctrl, ns, cmd.nsid))
206 return -EINVAL;
207
208 memset(&c, 0, sizeof(c));
209 c.common.opcode = cmd.opcode;
210 c.common.flags = cmd.flags;
211 c.common.nsid = cpu_to_le32(cmd.nsid);
212 c.common.cdw2[0] = cpu_to_le32(cmd.cdw2);
213 c.common.cdw2[1] = cpu_to_le32(cmd.cdw3);
214 c.common.cdw10 = cpu_to_le32(cmd.cdw10);
215 c.common.cdw11 = cpu_to_le32(cmd.cdw11);
216 c.common.cdw12 = cpu_to_le32(cmd.cdw12);
217 c.common.cdw13 = cpu_to_le32(cmd.cdw13);
218 c.common.cdw14 = cpu_to_le32(cmd.cdw14);
219 c.common.cdw15 = cpu_to_le32(cmd.cdw15);
220
221 if (cmd.timeout_ms)
222 timeout = msecs_to_jiffies(cmd.timeout_ms);
223
224 status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,
225 nvme_to_user_ptr(cmd.addr), cmd.data_len,
226 nvme_to_user_ptr(cmd.metadata), cmd.metadata_len,
227 0, &result, timeout);
228
229 if (status >= 0) {
230 if (put_user(result, &ucmd->result))
231 return -EFAULT;
232 }
233
234 /*
235 * Keep alive commands interval on the host should be updated
236 * when KATO is modified by Set Features commands.
237 */
238 if (!status && c.common.opcode == nvme_admin_set_features &&
> 239 ((u8)c.common.cdw10 & 0xFF) == NVME_FEAT_KATO) {
240 /* ms -> s */
> 241 unsigned int new_kato = DIV_ROUND_UP(c.common.cdw11, 1000);
242
243 dev_info(ctrl->device,
244 "keep alive commands interval on the host is updated from %u milliseconds to %u milliseconds\n",
245 ctrl->kato * 1000 / 2, new_kato * 1000 / 2);
246 nvme_stop_keep_alive(ctrl);
247 ctrl->kato = new_kato;
248 nvme_start_keep_alive(ctrl);
249 }
250
251 return status;
252 }
253

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx