linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V7 0/1] nvme: add verbose error logging
@ 2022-02-03  8:11 Chaitanya Kulkarni
  2022-02-03  8:11 ` [PATCH V7 1/1] nvme-core: Add " Chaitanya Kulkarni
  2022-02-16  8:01 ` [PATCH V7 0/1] nvme: add " Christoph Hellwig
  0 siblings, 2 replies; 10+ messages in thread
From: Chaitanya Kulkarni @ 2022-02-03  8:11 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, kbusch, sagi, Chaitanya Kulkarni

From: Chaitanya Kulkarni <kch@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



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

end of thread, other threads:[~2022-02-16 17:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-03  8:11 [PATCH V7 0/1] nvme: add verbose error logging Chaitanya Kulkarni
2022-02-03  8:11 ` [PATCH V7 1/1] nvme-core: Add " Chaitanya Kulkarni
2022-02-03  8:22   ` Chaitanya Kulkarni
2022-02-03 13:37     ` Sagi Grimberg
2022-02-03 13:43       ` Chaitanya Kulkarni
2022-02-09  7:51   ` Christoph Hellwig
2022-02-11 17:32     ` Alan Adamson
2022-02-16  8:01 ` [PATCH V7 0/1] nvme: add " Christoph Hellwig
2022-02-16  9:19   ` Chaitanya Kulkarni
2022-02-16 17:12     ` Alan Adamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).