[PATCH V8 0/4] nvme-core: timeout related fixes and cleanup

* [PATCH V8 0/4] nvme-core: timeout related fixes and cleanup
@ 2020-11-10  0:33 Chaitanya Kulkarni
  2020-11-10  0:33 ` [PATCH V8 1/4] nvme: centralize setting req timeout Chaitanya Kulkarni
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Chaitanya Kulkarni @ 2020-11-10  0:33 UTC (permalink / raw)
  To: linux-nvme; +Cc: kbusch, sagi, Chaitanya Kulkarni, hch

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 4581 bytes --]

Hi,

This patch series uses NVME_IO_TIMEOUT for the sync request submission
and user request submission when the timeout is not specified by the 
caller and request queue data is set to NULL which is true for admin 
queue.

Also in this version, I've added timeout values setting for the NVMeOF
passthru controller given that passthru VUCs (both admin and I/O VUCs)
can differ in execution time than general-purpose admin and I/O command
set, following is the detailed scenario :-

Consider NVMeOF target is setup in the passthru mode and both target
and host modules are loaded with the default timeout values present
in the nvme-core.

1. User issues nvme cmd from the host to the target of nvme-cli where
   timeout is taken from the controller log page  e.g. :-
   # nvme io-passthru --timeout=XXX ... 
   OR  
   # nvme admin-passthru --timeout=XXX ... 
   The timeout value in the above example is greater than the default
   timeout value present in host-core for both on host machine and the 
   target machine.
2. The host-core on the host side will set the timeout for the host
   request and issues NVMe cmd over transport.
3. In the absence of [1] target will set the timeout value which is
   default timeout and that is smaller than the timeout set on the host
   side.
4. Due to lack of timeout value not passed over the transport leading
   to smaller timeout value for the request on the target than the host
   side, target side command will timeout.

   This breaks the scenario where the same command with timeout value
   from nvme-cli is able to execute with success when issued to NVMe
   PCIe ctrl but will fail when it is executed on the NVMeOF Passthru
   ctrl.

Regards,
Chaitanya

[1] nvmet: add passthru admin timeout value attr
    nvmet: add passthru io timeout value attr

* Changes from V7 :-

1. Remove conditional operators.
2. Pass 0 instead of NVME_ADMIN_TIMEOUT for __nvme_submit_sync_cmd()
   when calling from nvme_sec_submit().

* Changes from V6 :-

1. Remove the timeout parameter for nvme_alloc_request() and keep the
   timeout assignment in the caller.
2. Remove the last patch for centralizing the req end_io_data.
3. Adjust the code for passthru to reflect new code changes.
4. Update the commit log of relative patches and add reviews.
5. Get rid of the locking for admin and io timeout for target
   passthru.

* Changes from V5:-

1. Rebase on the latest nvme-5.10.
2. Centralize setting up the request->timeout value into
   nvme_alloc_request().
3. Centralize setting up the request->end_io_data value into
   nvme_alloc_request().
4. Update the NVMeOF passthru nvmet_passthru_execute_cmd() such that
   it relfect nvme_alloc_request() changes from the first patch when
   handling io and admin timeouts.
5. Move nvme_req_set_default_timeout() to host/core.c since there is
   only one caller now.

* Changes from V4:-

1. Rebase and retest on nvme-5.10.

* Changes from V3:-

1. Rebase and retest on nvme-5.10.
2. Update the cover-letter with detailed scenario why NVMeOF target
   passthru is needed.
3. Wrap the line under 80 char for the last patch.
4. Change the nvme_default_timeout() -> nvme_set_req_default_timeout()
   such that it will not take timeout argument.

* Changes from V2:-

1. Introduce nvme_defalt_timeout() helper and use it in host/core.c.
2. Use nvme_default_timeout() in the lightnvme.c

* Changes from V1:-

1. Instead of using qid to decide IO or ADMIN timeout use request
   queue's queuedata whch we only set for non admin queue
   __nvme_submit_sync_cmd().
2. Add second patch to set IO timeout for nvme_submit_user_cmd().
3. Set the NVMeOF passthru ctrl timeout values with default values from
   nvme-core module.
4. Add admin and I/O timeout configfs attributes for NVMeOF passthru
   controller.       

Chaitanya Kulkarni (4):
  nvme: centralize setting req timeout
  nvmet: add passthru admin timeout value attr
  nvmet: add passthru io timeout value attr
  nvme: use consistent macro name for timeout

 drivers/nvme/host/core.c       | 15 ++++++++---
 drivers/nvme/host/fc.c         |  2 +-
 drivers/nvme/host/lightnvm.c   |  3 ++-
 drivers/nvme/host/nvme.h       |  2 +-
 drivers/nvme/host/pci.c        |  6 ++---
 drivers/nvme/host/rdma.c       |  2 +-
 drivers/nvme/host/tcp.c        |  2 +-
 drivers/nvme/target/configfs.c | 48 ++++++++++++++++++++++++++++++++++
 drivers/nvme/target/loop.c     |  2 +-
 drivers/nvme/target/nvmet.h    |  2 ++
 drivers/nvme/target/passthru.c |  7 +++++
 11 files changed, 77 insertions(+), 14 deletions(-)

-- 
2.22.1

[-- Attachment #2: Type: text/plain, Size: 158 bytes --]

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread