[PATCH V9 0/1] nvme: allow passthru cmd error logging

* [PATCH V9 0/1] nvme: allow passthru cmd error logging
@ 2024-01-30  0:19 Alan Adamson
  2024-01-30  0:19 ` [PATCH V9 1/1] " Alan Adamson
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Adamson @ 2024-01-30  0:19 UTC (permalink / raw)
  To: linux-nvme; +Cc: alan.adamson, kch, kbusch, hch, sagi

In nvme_end_req() we only log errors which are for non-passthru
commands. Add a helper function nvme_log_err_passthru() that allows us
to log error for passthru commands by decoding cdw10-cdw15 values of
nvme command.

Below is short testlog :-

* Admin Passsthru error log off, no errors are printed
* Admin Passsthru error log on, errors are printed
* IO Passsthru error log off, no errors are printed
* IO Passsthru error log on, errors are printed

v9:
- Move logging_enabled flag in device structure to nvme_ctrl and nvme_ns structures.
- Create seperate show/store functions for nvme_dev_attrs and nvme_ns_attrs.

v8:
- Add a logging_enabled flag to device structure.
- Add a passthru_err_log_enabled sysfs attribute for namespaces to
  allow logging of passthru IO commands.

v7:
- Changed attribute/flag to passthru_err_log_enabled.
- Use kstrtobool rather than kstrtoint.

v6:
- Rebase, retest nvme-6.5 and add test log for admin and I/O
  passthru error log.

v5:
- Trim down code in the nvme_log_error_passthrough().
  Use following to get the disk name as an arg to
   pr_err_ratelimited() :-
        ns ? ns->disk->disk_name : dev_name(nr->ctrl->device),
  Use following to get the admin vs I/O opcode string as an arg to
  pr_err_ratelimited() :-
        ns ? nvme_get_opcode_str(nr->cmd->common.opcode) :
             nvme_get_admin_opcode_str(nr->cmd->common.opcode),
- Rename nvme_log_error_passthrough() -> nvme_log_err_passthru().
- Remove else and return directly in nvme_passthru_err_log_show().
- Generate error on invalid values of the passthru_enable variable
  in nvme_passthru_log_store().
- Rename passthrough -> passthru.
- Rename sysfs attr from passthru_admin_err_logging -> passthru_log_err.

v4:
- Change sysfs attribute to passthru_admin_err_logging
- Only log passthrough admin commands.  IO passthrough commands will
  always be logged.

v3:
- Log a passthrough specific message that dumps CDW* contents.
- Enable/disable vis sysfs rather than debugfs.

v2:
- Included Pankaj Raghav's patch 'nvme: ignore starting sector while
  error logging for passthrough requests'
  with a couple changes.
- Moved error_logging flag to nvme_ctrl structure
- The entire nvme-debugfs.c does not need to be guarded by
  #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS.
- Use IS_ENABLED((CONFIG_NVME_ERROR_LOGGING_DEBUG_FS)) to determine if
  error logging should be initialized.
- Various other nits.

Testing
-------
* Admin Passsthru error log off, no errors are printed :-

[root@localhost ~]# echo 0 > /sys/class/nvme/nvme0/passthru_err_log_enabled
[root@localhost ~]# nvme telemetry-log -o /tmp/test /dev/nvme0
[root@localhost ~]# dmesg
[root@localhost ~]

* Admin Passsthru error log on, errors are printed :-

[root@localhost ~]# echo 1 > /sys/class/nvme/nvme0/passthru_err_log_enabled
[root@localhost ~]# nvme telemetry-log -o /tmp/test /dev/nvme0
[root@localhost ~]# dmesg
[ 2364.008105] nvme0: Get Log Page(0x2), Invalid Field in Command (sct 0x0 / sc 0x2) DNR cdw10=0x7f0107 cdw11=0x0 cdw12=0x0 cdw13=0x0 cdw14=0x0 cdw15=0x0

* IO Passsthru error log off, errors are not printed :-

[root@localhost ~]# echo 0 > /sys/class/nvme/nvme0/nvme0n1/passthru_err_log_enabled
[root@localhost ~]# nvme write-zeroes -n 1 -s 0x200000 -c 10 /dev/nvme0
NVMe status: LBA Out of Range: The command references an LBA that exceeds the size of the namespace(0x4080)
[root@localhost ~]# dmesg
[ 3131.602986] nvme nvme0: using deprecated NVME_IOCTL_IO_CMD ioctl on the char device!
[root@localhost ~]#

* IO Passsthru error log on, errors are printed :-

[root@localhost ~]# echo 1 > /sys/class/nvme/nvme0/nvme0n1/passthru_err_log_enabled
[root@localhost ~]# nvme write-zeroes -n 1 -s 0x200000 -c 10 /dev/nvme0
NVMe status: LBA Out of Range: The command references an LBA that exceeds the size of the namespace(0x4080)
[root@localhost ~]# dmesg
[ 2944.910393] nvme nvme0: using deprecated NVME_IOCTL_IO_CMD ioctl on the char device!
[ 2944.910423] nvme0n1: Write Zeroes(0x8), LBA Out of Range (sct 0x0 / sc 0x80) DNR cdw10=0x200000 cdw11=0x0 cdw12=0xa cdw13=0x0 cdw14=0x0 cdw15=0x0
[root@localhost ~]#

Alan Adamson (1):
  nvme: allow passthru cmd error logging

 drivers/nvme/host/core.c  | 59 +++++++++++++++++++++++++++++++++++----
 drivers/nvme/host/nvme.h  |  3 +-
 drivers/nvme/host/sysfs.c | 57 +++++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+), 7 deletions(-)

-- 
2.39.3

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