Skip to contentSkip to conversation

nvme: Add fault injection feature

PATCH Superseded Thomas Tai 3 participants 8 messages Jan 24, 2018 – Jan 29, 2018

Subsystems (3)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Thomas Tai

author
#0 lore raw

v2 cover nvme: Add fault injection feature

Linux's fault injection framework provides a systematic way to support error injection via debugfs in the /sys/kernel/debug directory. This patch uses the framework to add error injection to NVMe driver.

Change log since last version:
V1:
1. move error injection point into nvme_end_request()
2. Add expected result in the commit log

Thank you,
Thomas

author
#1 lore raw

PATCH v2 1/1 nvme: Add fault injection feature

Linux's fault injection framework provides a systematic way to support error injection via debugfs in the /sys/kernel/debug directory. This patch uses the framework to add error injection to NVMe driver. The fault injection source code is stored in a separate file and only linked if CONFIG_FAULT_INJECTION_DEBUG_FS kernel config is selected.

Once the error injection is enabled, NVME_SC_INVALID_OPCODE with no retry will be injected into the nvme_end_request. Following example shows how to inject an error.

First, enable CONFIG_FAULT_INJECTION_DEBUG_FS kernel config, recompile the kernel. After booting up the kernel, do the following.

Example:

mount /dev/nvme0n1 /mnt
echo 1 > /sys/kernel/debug/nvme0n1/fault_inject/times echo 100 > /sys/kernel/debug/nvme0n1/fault_inject/probability cp a.file /mnt

Expected Result:

cp: cannot stat ?/mnt/a.file?: Input/output error

Message from dmesg:

FAULT_INJECTION: forcing a failure.
name fault_inject, interval 1, probability 100, space 0, times 1

CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.15.0-rc8+ #2
Hardware name: innotek GmbH VirtualBox/VirtualBox,
BIOS VirtualBox 12/01/2006
Call Trace:
  <IRQ>
  dump_stack+0x5c/0x7d
  should_fail+0x148/0x170
  nvme_should_fail+0x2f/0x50 [nvme_core]
  nvme_process_cq+0xe7/0x1d0 [nvme]
  nvme_irq+0x1e/0x40 [nvme]
  __handle_irq_event_percpu+0x3a/0x190
  handle_irq_event_percpu+0x30/0x70
  handle_irq_event+0x36/0x60
  handle_fasteoi_irq+0x78/0x120
  handle_irq+0xa7/0x130
  ? tick_irq_enter+0xa8/0xc0
  do_IRQ+0x43/0xc0
  common_interrupt+0xa2/0xa2
  </IRQ>
RIP: 0010:native_safe_halt+0x2/0x10
RSP: 0018:ffffffff82003e90 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffdd
RAX: ffffffff817a10c0 RBX: ffffffff82012480 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 000000008e38ce64 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff82012480
R13: ffffffff82012480 R14: 0000000000000000 R15: 0000000000000000
  ? __sched_text_end+0x4/0x4
  default_idle+0x18/0xf0
  do_idle+0x150/0x1d0
  cpu_startup_entry+0x6f/0x80
  start_kernel+0x4c4/0x4e4
  ? set_init_arg+0x55/0x55
  secondary_startup_64+0xa5/0xb0
  print_req_error: I/O error, dev nvme0n1, sector 9240

EXT4-fs error (device nvme0n1): ext4_find_entry:1436:
inode #2: comm cp: reading directory lblock 0

Signed-off-by Thomas Tai <thomas.tai at oracle.com>
Reviewed-by Eric Saint-Etienne <eric.saint.etienne at oracle.com>
Signed-off-by Karl Volz <karl.volz at oracle.com>
---
 Documentation/fault-injection/fault-injection.txt |  5 ++
 drivers/nvme/host/Makefile                        |  1 +
 drivers/nvme/host/core.c                          |  2 +
 drivers/nvme/host/fault_inject.c                  | 67 +++++++++++++++++++++++
 drivers/nvme/host/nvme.h                          | 25 +++++++++
 5 files changed, 100 insertions(+)
 create mode 100644 drivers/nvme/host/fault_inject.c
Documentation/fault-injection/fault-injection.txt +5 -0
index 918972b..db547c2 100644
@@ -30,6 +30,11 @@ o fail_mmc_request
   injects MMC data errors on devices permitted by setting
   debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
 
+o NVMe fault injection
+
+  inject NVMe NVME_SC_INVALID_OPCODE with no retry on devices permitted
+  by setting debugfs entries under /sys/kernel/debug/nvme*/fault_inject
+
 Configure fault-injection capabilities behavior
 -----------------------------------------------
 
drivers/nvme/host/Makefile +1 -0
index a25fd43..d38131e 100644
@@ -8,6 +8,7 @@ obj-$(CONFIG_NVME_FC)			+= nvme-fc.o
 nvme-core-y				:= core.o
 nvme-core-$(CONFIG_NVME_MULTIPATH)	+= multipath.o
 nvme-core-$(CONFIG_NVM)			+= lightnvm.o
+nvme-core-$(CONFIG_FAULT_INJECTION_DEBUG_FS)	+= fault_inject.o
 
 nvme-y					+= pci.o
 
drivers/nvme/host/core.c +2 -0
index 839650e..49fa6da 100644
@@ -2951,6 +2951,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	if (new)
 		nvme_mpath_add_disk(ns->head);
 	nvme_mpath_add_disk_links(ns);
+	nvme_fault_inject_init(ns);
 	return;
  out_unlink_ns:
 	mutex_lock(&ctrl->subsys->lock);
@@ -2969,6 +2970,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 	if (test_and_set_bit(NVME_NS_REMOVING, &ns->flags))
 		return;
 
+	nvme_fault_inject_fini(ns);
 	if (ns->disk && ns->disk->flags & GENHD_FL_UP) {
 		nvme_mpath_remove_disk_links(ns);
 		sysfs_remove_group(&disk_to_dev(ns->disk)->kobj,
drivers/nvme/host/fault_inject.c new file +67 -0

67 lines not on this page · raw message

drivers/nvme/host/nvme.h +25 -0
index a00eabd..a3c47d8 100644
@@ -21,6 +21,7 @@
 #include <linux/blk-mq.h>
 #include <linux/lightnvm.h>
 #include <linux/sed-opal.h>
+#include <linux/fault-inject.h>
 
 extern unsigned int nvme_io_timeout;
 #define NVME_IO_TIMEOUT	(nvme_io_timeout * HZ)
@@ -257,6 +258,13 @@ struct nvme_ns_head {
 	int			instance;
 };
 
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+struct nvme_fault_inject {
+	struct fault_attr attr;
+	struct dentry *parent;
+};
+#endif
+
 struct nvme_ns {
 	struct list_head list;
 
@@ -278,6 +286,11 @@ struct nvme_ns {
 #define NVME_NS_REMOVING 0
 #define NVME_NS_DEAD     1
 	u16 noiob;
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+	struct nvme_fault_inject fault_inject;
+#endif
+
 };
 
 struct nvme_ctrl_ops {
@@ -296,6 +309,16 @@ struct nvme_ctrl_ops {
 	int (*reinit_request)(void *data, struct request *rq);
 };
 
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+void nvme_fault_inject_init(struct nvme_ns *ns);
+void nvme_fault_inject_fini(struct nvme_ns *ns);
+void nvme_should_fail(struct request *req);
+#else
+static inline void nvme_fault_inject_init(struct nvme_ns *ns) {}
+static inline void nvme_fault_inject_fini(struct nvme_ns *ns) {}
+static inline void nvme_should_fail(struct request *req) {}
+#endif
+
 static inline bool nvme_ctrl_ready(struct nvme_ctrl *ctrl)
 {
 	u32 val = 0;
@@ -332,6 +355,8 @@ static inline void nvme_end_request(struct request *req, __le16 status,
 
 	rq->status = le16_to_cpu(status) >> 1;
 	rq->result = result;
+	/* inject error when permitted by fault injection framework */
+	nvme_should_fail(req);
 	blk_mq_complete_request(req);
 }
 
Signature · 1 line
1.8.3.1
maintainer
#2 lore raw

PATCH v2 1/1 nvme: Add fault injection feature

Quoted 6 lines
drivers/nvme/host/fault_inject.c new file +5 -0
+void nvme_should_fail(struct request *req)
+{
+	struct gendisk *disk = req->rq_disk;
+	struct nvme_ns *ns = NULL;
+

drivers/nvme/host/fault_inject.c line 65

Quoted 10 lines
drivers/nvme/host/fault_inject.c new file +10 -0
+	/*
+	 * make sure this request is coming from a valid namespace
+	 */
+	if (!disk)
+		return;
+
+	ns = disk->private_data;
+	if (ns && should_fail(&ns->fault_inject.attr, 1))
+		/* inject the error status, do not retry */
+		nvme_req(req)->status =	NVME_SC_INVALID_OPCODE | NVME_SC_DNR;

I think it can be useful to control the status code (and DNR) as stuff like request retries and multipathing take it into account..

author
#3 lore raw

PATCH v2 1/1 nvme: Add fault injection feature

Quoted 11 lines · Sagi Grimberg
Quoted 11 lines
(quoted diff) +10 -0
+void nvme_should_fail(struct request *req)
+{
+??? struct gendisk *disk = req->rq_disk;
+??? struct nvme_ns *ns = NULL;
+
+??? /*
+???? * make sure this request is coming from a valid namespace
+???? */
+??? if (!disk)
+??????? return;
Quoted 8 lines
Quoted 5 lines

+
+??? ns = disk->private_data;
+??? if (ns && should_fail(&ns->fault_inject.attr, 1)) +??????? /* inject the error status, do not retry */
+??????? nvme_req(req)->status =??? NVME_SC_INVALID_OPCODE | NVME_SC_DNR;

I think it can be useful to control the status code (and DNR) as stuff like request retries and multipathing take it into account..

Hi Sagi,
Thank you for your suggestion. I will add an options in the debugfs to control the status code and the DNR setting.

Thanks,
Thomas

author
#4 lore raw

PATCH v2 1/1 nvme: Add fault injection feature

In reply to Sagi Grimberg
Quoted 11 lines · Sagi Grimberg
Quoted 11 lines
(quoted diff) +10 -0
+void nvme_should_fail(struct request *req)
+{
+??? struct gendisk *disk = req->rq_disk;
+??? struct nvme_ns *ns = NULL;
+
+??? /*
+???? * make sure this request is coming from a valid namespace
+???? */
+??? if (!disk)
+??????? return;
Quoted 8 lines
Quoted 5 lines

+
+??? ns = disk->private_data;
+??? if (ns && should_fail(&ns->fault_inject.attr, 1)) +??????? /* inject the error status, do not retry */
+??????? nvme_req(req)->status =??? NVME_SC_INVALID_OPCODE | NVME_SC_DNR;

I think it can be useful to control the status code (and DNR) as stuff like request retries and multipathing take it into account..

Hi Sagi,
Would you be more specific about the multipathing setting?

Thank you,
Thomas

maintainer
#5 lore raw

PATCH v2 1/1 nvme: Add fault injection feature

Quoted 3 lines

Hi Sagi,
Would you be more specific about the multipathing setting?

Every retry-able status code would trigger path-failover (see nvme_error_status).

#6 lore raw

v2 cover nvme: Add fault injection feature

In reply to Thomas Tai
Quoted 3 lines · Thomas Tai

Linux's fault injection framework provides a systematic way to support error injection via debugfs in the /sys/kernel/debug directory. This patch uses the framework to add error injection to NVMe driver.

Care to expand on the use cases a bit more? Especially on what this buys us vs block layer error injection?

author
#7 lore raw

v2 cover nvme: Add fault injection feature

Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · Thomas Tai

Linux's fault injection framework provides a systematic way to support error injection via debugfs in the /sys/kernel/debug directory. This patch uses the framework to add error injection to NVMe driver.

Care to expand on the use cases a bit more? Especially on what this buys us vs block layer error injection?

Hi Christoph,

Thank you for your questions. The reason for adding error injection to NVMe is ensure we're exercising error handling specifically for nvme code path. The block layer error injection happens at the block layer while the nvme error injection happens in the driver layer.

Following is the simplified diagram for those who are interested, where *** denote block layer error injection point.
+++ denote nvme error injection point.

.------------------------------ [ submit IO ] ---.

  |  Block layer                       |           |
  |                                    |           |
  |            ***generic_make_request |           |
  |                                    |           |
  |                                    v           |
  |                                  .---.         |
  | [ Submission/Completion      ]   |   |         |
  | [ Staging (Merge, Reorder, ) ]   .---. Req que |
  | [ Fairness Scheduling        ]   |   |         |
  | [ IO Accounting              ]   .---.         |
  |                                  |   |         |
  |                                  '---'         |
  |                                    |           |
  '----------------------------------- | ----------'
                                       v
  .------------------------------------------------.
  | Block device specific driver                   |
  |                                                |
  |     +++nvme_end_request                        |
  '------------------------------------------------'
            ^                          |
            | Status/Completion        |
            | Interrupt                |
            |                          v

.------------------------------------------------.
| Hardware device |
'------------------------------------------------'

Thanks,
Thomas