[PATCH V7 0/1] nvme: add verbose error logging

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch at nvidia.com>

Hi,

I spent sometime on reviwing this patch, thought it will be
helpeful if I fix things while reviwing it along with
Christoph's comments.

Follwing is the original cover-letter:-

This patch improves logging for NVMe errors. Currently, we only get
a vague idea as to why commands fail since only the block layer status
is captured on error. This patch allows us to see why a command was
failed by the controller. This is very useful when debugging problems
in the field.

An example of an improved logged error:

[  183.333734] nvme0n1: Read(0x2) @ LBA 0, 1 blocks, Unrecovered Read Error (sct 0x2 / sc 0x81) DNR

[  227.767945] nvme0: Activate Firmware(0x10), Invalid Field in Command (sct 0x0 / sc 0x2) DNR

-ck

V7:
- Remove local variables from helpers nvme_get_error_status_str()
  nvme_get_opcode_str(), nvme_get_admin_opcode_str().
- Fix commit message of patch and remove the example since 
  cover-letter already has it.
- Un-export helper functions.
- Remove forward declaration of nvme_log_error().
- Move nvme_log_error() before its caller.
- Remove local variables initialization admin_op_str and op_str
  as they are getting overwritten by respective functions in
  nvme_log_error().
- Rename the error.c file to constants.c
- Fix the return type of the stubs
  nvme_get_error_status_str(u16 status),
  nvme_get_opcode_str(u8 opcode), and
  nvme_get_admin_opcode_str(u8 opcode) to const unsigned char *
  to match the what have in the constants.c.
- Add tabs before the comments for pr_err_ratelimited() functions
  arguments so that it will be consistent for nvme and admin commands.

V6:
- Create helpers that are stubbed out if
  CONFIG_NVME_VERBOSE_ERRORS is not set.
- Couple more nits.

V5:
- Change nvme_ops[] and nvme_admin_ops[] to a sparse array where the
  opcode is used as an index into the array.
- Various other nits.

V4:
- Adding logging for admin commands.
- Display NVMe Command Opcode value rather than generic block opcode.
- Various other nits.

V3:
- Don't populate nvme_errors[] array with NULL strings.

V2:
- Change nvme_errors[] to a sparse array where the status is used as
  an index into the array.

- Change pr_err() to pr_err_ratelimited().

- By enabling CONFIG_NVME_VERBOSE_ERRORS, the verbose status error is
  displayed.  If it is not enabled, then a message will still be
  displayed, but without the verbose status.

- Remove call to nvme_error_status() when determining whether to call
  nvme_error_log(). Speeds up the fast path just a bit.

Chaitanya Kulkarni (1):
  nvme-core: Add verbose error logging

 drivers/nvme/host/Kconfig     |   8 ++
 drivers/nvme/host/Makefile    |   2 +-
 drivers/nvme/host/constants.c | 185 ++++++++++++++++++++++++++++++++++
 drivers/nvme/host/core.c      |  34 +++++++
 drivers/nvme/host/nvme.h      |  19 ++++
 include/linux/nvme.h          |   1 +
 6 files changed, 248 insertions(+), 1 deletion(-)
 create mode 100644 drivers/nvme/host/constants.c

-- 
2.29.0