* [PATCH v2 0/3] nvme: enable to inject errors into admin commands
@ 2019-06-02 13:50 Akinobu Mita
2019-06-02 13:50 ` [PATCH v2 1/3] nvme: prepare for fault injection " Akinobu Mita
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Akinobu Mita @ 2019-06-02 13:50 UTC (permalink / raw)
Currenlty fault injection support for nvme only enables to inject errors
into the commands submitted to I/O queues.
This enables to inject errors into the commands submitted to the admin
queue.
It is useful to test error handling in the controller initialization.
# echo 100 > /sys/kernel/debug/nvme0/fault_inject/probability
# echo 1 > /sys/kernel/debug/nvme0/fault_inject/times
# echo 10 > /sys/kernel/debug/nvme0/fault_inject/space
# nvme reset /dev/nvme0
# dmesg
...
nvme nvme0: Could not set queue count (16385)
nvme nvme0: IO queues not created
* v2
- rename the argument 'name' to 'dev_name'
- add Reviewed-by tags
- add documentation for nvme fault injection
Akinobu Mita (3):
nvme: prepare for fault injection into admin commands
nvme: enable to inject errors into admin commands
Documentation: nvme: add an example for nvme fault injection
.../fault-injection/nvme-fault-injection.txt | 56 ++++++++++++++++++++++
drivers/nvme/host/core.c | 7 ++-
drivers/nvme/host/fault_inject.c | 39 +++++++--------
drivers/nvme/host/nvme.h | 36 ++++++++------
4 files changed, 102 insertions(+), 36 deletions(-)
Cc: Thomas Tai <thomas.tai at oracle.com>
Cc: Keith Busch <kbusch at kernel.org>
Cc: Jens Axboe <axboe at fb.com>
Cc: Christoph Hellwig <hch at lst.de>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Minwoo Im <minwoo.im.dev at gmail.com>
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] nvme: prepare for fault injection into admin commands
2019-06-02 13:50 [PATCH v2 0/3] nvme: enable to inject errors into admin commands Akinobu Mita
@ 2019-06-02 13:50 ` Akinobu Mita
2019-06-02 22:49 ` Chaitanya Kulkarni
2019-06-02 13:50 ` [PATCH v2 2/3] nvme: enable to inject errors " Akinobu Mita
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Akinobu Mita @ 2019-06-02 13:50 UTC (permalink / raw)
Currenlty fault injection support for nvme only enables to inject errors
into the commands submitted to I/O queues.
In preparation for fault injection into the admin commands, this makes
the helper functions independent of struct nvme_ns.
Cc: Thomas Tai <thomas.tai at oracle.com>
Cc: Keith Busch <kbusch at kernel.org>
Cc: Jens Axboe <axboe at fb.com>
Cc: Christoph Hellwig <hch at lst.de>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Minwoo Im <minwoo.im.dev at gmail.com>
Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
---
* v2
- rename the argument 'name' to 'dev_name'
- add Reviewed-by tags
drivers/nvme/host/core.c | 4 ++--
drivers/nvme/host/fault_inject.c | 34 ++++++++++++++++++----------------
drivers/nvme/host/nvme.h | 20 ++++++++++++--------
3 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 26c8b59..9fca8457 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3698,7 +3698,7 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
nvme_mpath_add_disk(ns, id);
- nvme_fault_inject_init(ns);
+ nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name);
kfree(id);
return 0;
@@ -3723,7 +3723,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);
+ nvme_fault_inject_fini(&ns->fault_inject);
if (ns->disk && ns->disk->flags & GENHD_FL_UP) {
del_gendisk(ns->disk);
blk_cleanup_queue(ns->queue);
diff --git a/drivers/nvme/host/fault_inject.c b/drivers/nvme/host/fault_inject.c
index 4cfd2c9..18ad983 100644
--- a/drivers/nvme/host/fault_inject.c
+++ b/drivers/nvme/host/fault_inject.c
@@ -15,11 +15,10 @@ static DECLARE_FAULT_ATTR(fail_default_attr);
static char *fail_request;
module_param(fail_request, charp, 0000);
-void nvme_fault_inject_init(struct nvme_ns *ns)
+void nvme_fault_inject_init(struct nvme_fault_inject *fault_inj,
+ const char *dev_name)
{
struct dentry *dir, *parent;
- char *name = ns->disk->disk_name;
- struct nvme_fault_inject *fault_inj = &ns->fault_inject;
struct fault_attr *attr = &fault_inj->attr;
/* set default fault injection attribute */
@@ -27,20 +26,20 @@ void nvme_fault_inject_init(struct nvme_ns *ns)
setup_fault_attr(&fail_default_attr, fail_request);
/* create debugfs directory and attribute */
- parent = debugfs_create_dir(name, NULL);
+ parent = debugfs_create_dir(dev_name, NULL);
if (!parent) {
- pr_warn("%s: failed to create debugfs directory\n", name);
+ pr_warn("%s: failed to create debugfs directory\n", dev_name);
return;
}
*attr = fail_default_attr;
dir = fault_create_debugfs_attr("fault_inject", parent, attr);
if (IS_ERR(dir)) {
- pr_warn("%s: failed to create debugfs attr\n", name);
+ pr_warn("%s: failed to create debugfs attr\n", dev_name);
debugfs_remove_recursive(parent);
return;
}
- ns->fault_inject.parent = parent;
+ fault_inj->parent = parent;
/* create debugfs for status code and dont_retry */
fault_inj->status = NVME_SC_INVALID_OPCODE;
@@ -49,29 +48,32 @@ void nvme_fault_inject_init(struct nvme_ns *ns)
debugfs_create_bool("dont_retry", 0600, dir, &fault_inj->dont_retry);
}
-void nvme_fault_inject_fini(struct nvme_ns *ns)
+void nvme_fault_inject_fini(struct nvme_fault_inject *fault_inject)
{
/* remove debugfs directories */
- debugfs_remove_recursive(ns->fault_inject.parent);
+ debugfs_remove_recursive(fault_inject->parent);
}
void nvme_should_fail(struct request *req)
{
struct gendisk *disk = req->rq_disk;
- struct nvme_ns *ns = NULL;
+ struct nvme_fault_inject *fault_inject = NULL;
u16 status;
/*
* make sure this request is coming from a valid namespace
*/
- if (!disk)
- return;
+ if (disk) {
+ struct nvme_ns *ns = disk->private_data;
+
+ if (ns)
+ fault_inject = &ns->fault_inject;
+ }
- ns = disk->private_data;
- if (ns && should_fail(&ns->fault_inject.attr, 1)) {
+ if (fault_inject && should_fail(&fault_inject->attr, 1)) {
/* inject status code and DNR bit */
- status = ns->fault_inject.status;
- if (ns->fault_inject.dont_retry)
+ status = fault_inject->status;
+ if (fault_inject->dont_retry)
status |= NVME_SC_DNR;
nvme_req(req)->status = status;
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 53f0b24..c1658ed 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -323,14 +323,14 @@ struct nvme_ns_head {
#endif
};
-#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
struct nvme_fault_inject {
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
struct fault_attr attr;
struct dentry *parent;
bool dont_retry; /* DNR, do not retry */
u16 status; /* status code */
-};
#endif
+};
struct nvme_ns {
struct list_head list;
@@ -359,9 +359,7 @@ struct nvme_ns {
#define NVME_NS_ANA_PENDING 2
u16 noiob;
-#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
struct nvme_fault_inject fault_inject;
-#endif
};
@@ -382,12 +380,18 @@ struct nvme_ctrl_ops {
};
#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_fault_inject_init(struct nvme_fault_inject *fault_inj,
+ const char *dev_name);
+void nvme_fault_inject_fini(struct nvme_fault_inject *fault_inject);
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_fault_inject_init(struct nvme_fault_inject *fault_inj,
+ const char *dev_name)
+{
+}
+static inline void nvme_fault_inject_fini(struct nvme_fault_inject *fault_inj)
+{
+}
static inline void nvme_should_fail(struct request *req) {}
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] nvme: enable to inject errors into admin commands
2019-06-02 13:50 [PATCH v2 0/3] nvme: enable to inject errors into admin commands Akinobu Mita
2019-06-02 13:50 ` [PATCH v2 1/3] nvme: prepare for fault injection " Akinobu Mita
@ 2019-06-02 13:50 ` Akinobu Mita
2019-06-02 22:49 ` Chaitanya Kulkarni
2019-06-04 7:23 ` Christoph Hellwig
2019-06-02 13:50 ` [PATCH v2 3/3] Documentation: nvme: add an example for nvme fault injection Akinobu Mita
[not found] ` <CGME20190602135117epcas4p14feefbe2556a469008a7a499174e5b43@epcms2p1>
3 siblings, 2 replies; 10+ messages in thread
From: Akinobu Mita @ 2019-06-02 13:50 UTC (permalink / raw)
This enables to inject errors into the commands submitted to the admin
queue.
It is useful to test error handling in the controller initialization.
# echo 100 > /sys/kernel/debug/nvme0/fault_inject/probability
# echo 1 > /sys/kernel/debug/nvme0/fault_inject/times
# echo 10 > /sys/kernel/debug/nvme0/fault_inject/space
# nvme reset /dev/nvme0
# dmesg
...
nvme nvme0: Could not set queue count (16385)
nvme nvme0: IO queues not created
Cc: Thomas Tai <thomas.tai at oracle.com>
Cc: Keith Busch <kbusch at kernel.org>
Cc: Jens Axboe <axboe at fb.com>
Cc: Christoph Hellwig <hch at lst.de>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Minwoo Im <minwoo.im.dev at gmail.com>
Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
---
* v2
- add Reviewed-by tags
drivers/nvme/host/core.c | 3 +++
drivers/nvme/host/fault_inject.c | 5 ++---
drivers/nvme/host/nvme.h | 20 +++++++++++---------
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9fca8457..66d8199 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4093,6 +4093,7 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl);
void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
{
+ nvme_fault_inject_fini(&ctrl->fault_inject);
dev_pm_qos_hide_latency_tolerance(ctrl->device);
cdev_device_del(&ctrl->cdev, ctrl->device);
}
@@ -4188,6 +4189,8 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
dev_pm_qos_update_user_latency_tolerance(ctrl->device,
min(default_ps_max_latency_us, (unsigned long)S32_MAX));
+ nvme_fault_inject_init(&ctrl->fault_inject, dev_name(ctrl->device));
+
return 0;
out_free_name:
kfree_const(ctrl->device->kobj.name);
diff --git a/drivers/nvme/host/fault_inject.c b/drivers/nvme/host/fault_inject.c
index 18ad983..5868edf 100644
--- a/drivers/nvme/host/fault_inject.c
+++ b/drivers/nvme/host/fault_inject.c
@@ -60,14 +60,13 @@ void nvme_should_fail(struct request *req)
struct nvme_fault_inject *fault_inject = NULL;
u16 status;
- /*
- * make sure this request is coming from a valid namespace
- */
if (disk) {
struct nvme_ns *ns = disk->private_data;
if (ns)
fault_inject = &ns->fault_inject;
+ } else {
+ fault_inject = &nvme_req(req)->ctrl->fault_inject;
}
if (fault_inject && should_fail(&fault_inject->attr, 1)) {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index c1658ed..e710829 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -147,6 +147,15 @@ enum nvme_ctrl_state {
NVME_CTRL_DEAD,
};
+struct nvme_fault_inject {
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+ struct fault_attr attr;
+ struct dentry *parent;
+ bool dont_retry; /* DNR, do not retry */
+ u16 status; /* status code */
+#endif
+};
+
struct nvme_ctrl {
bool comp_seen;
enum nvme_ctrl_state state;
@@ -257,6 +266,8 @@ struct nvme_ctrl {
*/
struct thermal_zone_device *tzdev[9];
#endif
+
+ struct nvme_fault_inject fault_inject;
};
enum nvme_iopolicy {
@@ -323,15 +334,6 @@ struct nvme_ns_head {
#endif
};
-struct nvme_fault_inject {
-#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
- struct fault_attr attr;
- struct dentry *parent;
- bool dont_retry; /* DNR, do not retry */
- u16 status; /* status code */
-#endif
-};
-
struct nvme_ns {
struct list_head list;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] Documentation: nvme: add an example for nvme fault injection
2019-06-02 13:50 [PATCH v2 0/3] nvme: enable to inject errors into admin commands Akinobu Mita
2019-06-02 13:50 ` [PATCH v2 1/3] nvme: prepare for fault injection " Akinobu Mita
2019-06-02 13:50 ` [PATCH v2 2/3] nvme: enable to inject errors " Akinobu Mita
@ 2019-06-02 13:50 ` Akinobu Mita
2019-06-02 22:49 ` Chaitanya Kulkarni
[not found] ` <CGME20190602135117epcas4p14feefbe2556a469008a7a499174e5b43@epcms2p1>
3 siblings, 1 reply; 10+ messages in thread
From: Akinobu Mita @ 2019-06-02 13:50 UTC (permalink / raw)
This adds an example of how to inject errors into admin commands.
Cc: Thomas Tai <thomas.tai at oracle.com>
Cc: Keith Busch <kbusch at kernel.org>
Cc: Jens Axboe <axboe at fb.com>
Cc: Christoph Hellwig <hch at lst.de>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Minwoo Im <minwoo.im.dev at gmail.com>
Suggested-by: Thomas Tai <thomas.tai at oracle.com>
Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
---
* v2
- New patch from v2
.../fault-injection/nvme-fault-injection.txt | 56 ++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/Documentation/fault-injection/nvme-fault-injection.txt b/Documentation/fault-injection/nvme-fault-injection.txt
index 8fbf3bf..efcb339 100644
--- a/Documentation/fault-injection/nvme-fault-injection.txt
+++ b/Documentation/fault-injection/nvme-fault-injection.txt
@@ -114,3 +114,59 @@ R13: ffff88011a3c9680 R14: 0000000000000000 R15: 0000000000000000
cpu_startup_entry+0x6f/0x80
start_secondary+0x187/0x1e0
secondary_startup_64+0xa5/0xb0
+
+Example 3: Inject an error into the 10th admin command
+------------------------------------------------------
+
+echo 100 > /sys/kernel/debug/nvme0/fault_inject/probability
+echo 10 > /sys/kernel/debug/nvme0/fault_inject/space
+echo 1 > /sys/kernel/debug/nvme0/fault_inject/times
+nvme reset /dev/nvme0
+
+Expected Result:
+
+After NVMe controller reset, the reinitialization may or may not succeed.
+It depends on which admin command is actually forced to fail.
+
+Message from dmesg:
+
+nvme nvme0: resetting controller
+FAULT_INJECTION: forcing a failure.
+name fault_inject, interval 1, probability 100, space 1, times 1
+CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0-rc2+ #2
+Hardware name: MSI MS-7A45/B150M MORTAR ARCTIC (MS-7A45), BIOS 1.50 04/25/2017
+Call Trace:
+ <IRQ>
+ dump_stack+0x63/0x85
+ should_fail+0x14a/0x170
+ nvme_should_fail+0x38/0x80 [nvme_core]
+ nvme_irq+0x129/0x280 [nvme]
+ ? blk_mq_end_request+0xb3/0x120
+ __handle_irq_event_percpu+0x84/0x1a0
+ handle_irq_event_percpu+0x32/0x80
+ handle_irq_event+0x3b/0x60
+ handle_edge_irq+0x7f/0x1a0
+ handle_irq+0x20/0x30
+ do_IRQ+0x4e/0xe0
+ common_interrupt+0xf/0xf
+ </IRQ>
+RIP: 0010:cpuidle_enter_state+0xc5/0x460
+Code: ff e8 8f 5f 86 ff 80 7d c7 00 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 69 03 00 00 31 ff e8 62 aa 8c ff fb 66 0f 1f 44 00 00 <45> 85 ed 0f 88 37 03 00 00 4c 8b 45 d0 4c 2b 45 b8 48 ba cf f7 53
+RSP: 0018:ffffffff88c03dd0 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffdc
+RAX: ffff9dac25a2ac80 RBX: ffffffff88d53760 RCX: 000000000000001f
+RDX: 0000000000000000 RSI: 000000002d958403 RDI: 0000000000000000
+RBP: ffffffff88c03e18 R08: fffffff75e35ffb7 R09: 00000a49a56c0b48
+R10: ffffffff88c03da0 R11: 0000000000001b0c R12: ffff9dac25a34d00
+R13: 0000000000000006 R14: 0000000000000006 R15: ffffffff88d53760
+ cpuidle_enter+0x2e/0x40
+ call_cpuidle+0x23/0x40
+ do_idle+0x201/0x280
+ cpu_startup_entry+0x1d/0x20
+ rest_init+0xaa/0xb0
+ arch_call_rest_init+0xe/0x1b
+ start_kernel+0x51c/0x53b
+ x86_64_start_reservations+0x24/0x26
+ x86_64_start_kernel+0x74/0x77
+ secondary_startup_64+0xa4/0xb0
+nvme nvme0: Could not set queue count (16385)
+nvme nvme0: IO queues not created
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] nvme: prepare for fault injection into admin commands
2019-06-02 13:50 ` [PATCH v2 1/3] nvme: prepare for fault injection " Akinobu Mita
@ 2019-06-02 22:49 ` Chaitanya Kulkarni
0 siblings, 0 replies; 10+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-02 22:49 UTC (permalink / raw)
Looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 6/2/19 6:51 AM, Akinobu Mita wrote:
> Currenlty fault injection support for nvme only enables to inject errors
> into the commands submitted to I/O queues.
>
> In preparation for fault injection into the admin commands, this makes
> the helper functions independent of struct nvme_ns.
>
> Cc: Thomas Tai <thomas.tai at oracle.com>
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Jens Axboe <axboe at fb.com>
> Cc: Christoph Hellwig <hch at lst.de>
> Cc: Sagi Grimberg <sagi at grimberg.me>
> Cc: Minwoo Im <minwoo.im.dev at gmail.com>
> Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
> Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
> Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> * v2
> - rename the argument 'name' to 'dev_name'
> - add Reviewed-by tags
>
> drivers/nvme/host/core.c | 4 ++--
> drivers/nvme/host/fault_inject.c | 34 ++++++++++++++++++----------------
> drivers/nvme/host/nvme.h | 20 ++++++++++++--------
> 3 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 26c8b59..9fca8457 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3698,7 +3698,7 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
>
> nvme_mpath_add_disk(ns, id);
> - nvme_fault_inject_init(ns);
> + nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name);
> kfree(id);
>
> return 0;
> @@ -3723,7 +3723,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);
> + nvme_fault_inject_fini(&ns->fault_inject);
> if (ns->disk && ns->disk->flags & GENHD_FL_UP) {
> del_gendisk(ns->disk);
> blk_cleanup_queue(ns->queue);
> diff --git a/drivers/nvme/host/fault_inject.c b/drivers/nvme/host/fault_inject.c
> index 4cfd2c9..18ad983 100644
> --- a/drivers/nvme/host/fault_inject.c
> +++ b/drivers/nvme/host/fault_inject.c
> @@ -15,11 +15,10 @@ static DECLARE_FAULT_ATTR(fail_default_attr);
> static char *fail_request;
> module_param(fail_request, charp, 0000);
>
> -void nvme_fault_inject_init(struct nvme_ns *ns)
> +void nvme_fault_inject_init(struct nvme_fault_inject *fault_inj,
> + const char *dev_name)
> {
> struct dentry *dir, *parent;
> - char *name = ns->disk->disk_name;
> - struct nvme_fault_inject *fault_inj = &ns->fault_inject;
> struct fault_attr *attr = &fault_inj->attr;
>
> /* set default fault injection attribute */
> @@ -27,20 +26,20 @@ void nvme_fault_inject_init(struct nvme_ns *ns)
> setup_fault_attr(&fail_default_attr, fail_request);
>
> /* create debugfs directory and attribute */
> - parent = debugfs_create_dir(name, NULL);
> + parent = debugfs_create_dir(dev_name, NULL);
> if (!parent) {
> - pr_warn("%s: failed to create debugfs directory\n", name);
> + pr_warn("%s: failed to create debugfs directory\n", dev_name);
> return;
> }
>
> *attr = fail_default_attr;
> dir = fault_create_debugfs_attr("fault_inject", parent, attr);
> if (IS_ERR(dir)) {
> - pr_warn("%s: failed to create debugfs attr\n", name);
> + pr_warn("%s: failed to create debugfs attr\n", dev_name);
> debugfs_remove_recursive(parent);
> return;
> }
> - ns->fault_inject.parent = parent;
> + fault_inj->parent = parent;
>
> /* create debugfs for status code and dont_retry */
> fault_inj->status = NVME_SC_INVALID_OPCODE;
> @@ -49,29 +48,32 @@ void nvme_fault_inject_init(struct nvme_ns *ns)
> debugfs_create_bool("dont_retry", 0600, dir, &fault_inj->dont_retry);
> }
>
> -void nvme_fault_inject_fini(struct nvme_ns *ns)
> +void nvme_fault_inject_fini(struct nvme_fault_inject *fault_inject)
> {
> /* remove debugfs directories */
> - debugfs_remove_recursive(ns->fault_inject.parent);
> + debugfs_remove_recursive(fault_inject->parent);
> }
>
> void nvme_should_fail(struct request *req)
> {
> struct gendisk *disk = req->rq_disk;
> - struct nvme_ns *ns = NULL;
> + struct nvme_fault_inject *fault_inject = NULL;
> u16 status;
>
> /*
> * make sure this request is coming from a valid namespace
> */
> - if (!disk)
> - return;
> + if (disk) {
> + struct nvme_ns *ns = disk->private_data;
> +
> + if (ns)
> + fault_inject = &ns->fault_inject;
> + }
>
> - ns = disk->private_data;
> - if (ns && should_fail(&ns->fault_inject.attr, 1)) {
> + if (fault_inject && should_fail(&fault_inject->attr, 1)) {
> /* inject status code and DNR bit */
> - status = ns->fault_inject.status;
> - if (ns->fault_inject.dont_retry)
> + status = fault_inject->status;
> + if (fault_inject->dont_retry)
> status |= NVME_SC_DNR;
> nvme_req(req)->status = status;
> }
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 53f0b24..c1658ed 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -323,14 +323,14 @@ struct nvme_ns_head {
> #endif
> };
>
> -#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> struct nvme_fault_inject {
> +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> struct fault_attr attr;
> struct dentry *parent;
> bool dont_retry; /* DNR, do not retry */
> u16 status; /* status code */
> -};
> #endif
> +};
>
> struct nvme_ns {
> struct list_head list;
> @@ -359,9 +359,7 @@ struct nvme_ns {
> #define NVME_NS_ANA_PENDING 2
> u16 noiob;
>
> -#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> struct nvme_fault_inject fault_inject;
> -#endif
>
> };
>
> @@ -382,12 +380,18 @@ struct nvme_ctrl_ops {
> };
>
> #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_fault_inject_init(struct nvme_fault_inject *fault_inj,
> + const char *dev_name);
> +void nvme_fault_inject_fini(struct nvme_fault_inject *fault_inject);
> 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_fault_inject_init(struct nvme_fault_inject *fault_inj,
> + const char *dev_name)
> +{
> +}
> +static inline void nvme_fault_inject_fini(struct nvme_fault_inject *fault_inj)
> +{
> +}
> static inline void nvme_should_fail(struct request *req) {}
> #endif
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] nvme: enable to inject errors into admin commands
2019-06-02 13:50 ` [PATCH v2 2/3] nvme: enable to inject errors " Akinobu Mita
@ 2019-06-02 22:49 ` Chaitanya Kulkarni
2019-06-04 7:23 ` Christoph Hellwig
1 sibling, 0 replies; 10+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-02 22:49 UTC (permalink / raw)
Looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 6/2/19 6:51 AM, Akinobu Mita wrote:
> This enables to inject errors into the commands submitted to the admin
> queue.
>
> It is useful to test error handling in the controller initialization.
>
> # echo 100 > /sys/kernel/debug/nvme0/fault_inject/probability
> # echo 1 > /sys/kernel/debug/nvme0/fault_inject/times
> # echo 10 > /sys/kernel/debug/nvme0/fault_inject/space
> # nvme reset /dev/nvme0
> # dmesg
> ...
> nvme nvme0: Could not set queue count (16385)
> nvme nvme0: IO queues not created
>
> Cc: Thomas Tai <thomas.tai at oracle.com>
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Jens Axboe <axboe at fb.com>
> Cc: Christoph Hellwig <hch at lst.de>
> Cc: Sagi Grimberg <sagi at grimberg.me>
> Cc: Minwoo Im <minwoo.im.dev at gmail.com>
> Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
> Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
> Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> * v2
> - add Reviewed-by tags
>
> drivers/nvme/host/core.c | 3 +++
> drivers/nvme/host/fault_inject.c | 5 ++---
> drivers/nvme/host/nvme.h | 20 +++++++++++---------
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 9fca8457..66d8199 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4093,6 +4093,7 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl);
>
> void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
> {
> + nvme_fault_inject_fini(&ctrl->fault_inject);
> dev_pm_qos_hide_latency_tolerance(ctrl->device);
> cdev_device_del(&ctrl->cdev, ctrl->device);
> }
> @@ -4188,6 +4189,8 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> dev_pm_qos_update_user_latency_tolerance(ctrl->device,
> min(default_ps_max_latency_us, (unsigned long)S32_MAX));
>
> + nvme_fault_inject_init(&ctrl->fault_inject, dev_name(ctrl->device));
> +
> return 0;
> out_free_name:
> kfree_const(ctrl->device->kobj.name);
> diff --git a/drivers/nvme/host/fault_inject.c b/drivers/nvme/host/fault_inject.c
> index 18ad983..5868edf 100644
> --- a/drivers/nvme/host/fault_inject.c
> +++ b/drivers/nvme/host/fault_inject.c
> @@ -60,14 +60,13 @@ void nvme_should_fail(struct request *req)
> struct nvme_fault_inject *fault_inject = NULL;
> u16 status;
>
> - /*
> - * make sure this request is coming from a valid namespace
> - */
> if (disk) {
> struct nvme_ns *ns = disk->private_data;
>
> if (ns)
> fault_inject = &ns->fault_inject;
> + } else {
> + fault_inject = &nvme_req(req)->ctrl->fault_inject;
> }
>
> if (fault_inject && should_fail(&fault_inject->attr, 1)) {
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index c1658ed..e710829 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -147,6 +147,15 @@ enum nvme_ctrl_state {
> NVME_CTRL_DEAD,
> };
>
> +struct nvme_fault_inject {
> +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> + struct fault_attr attr;
> + struct dentry *parent;
> + bool dont_retry; /* DNR, do not retry */
> + u16 status; /* status code */
> +#endif
> +};
> +
> struct nvme_ctrl {
> bool comp_seen;
> enum nvme_ctrl_state state;
> @@ -257,6 +266,8 @@ struct nvme_ctrl {
> */
> struct thermal_zone_device *tzdev[9];
> #endif
> +
> + struct nvme_fault_inject fault_inject;
> };
>
> enum nvme_iopolicy {
> @@ -323,15 +334,6 @@ struct nvme_ns_head {
> #endif
> };
>
> -struct nvme_fault_inject {
> -#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> - struct fault_attr attr;
> - struct dentry *parent;
> - bool dont_retry; /* DNR, do not retry */
> - u16 status; /* status code */
> -#endif
> -};
> -
> struct nvme_ns {
> struct list_head list;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] Documentation: nvme: add an example for nvme fault injection
2019-06-02 13:50 ` [PATCH v2 3/3] Documentation: nvme: add an example for nvme fault injection Akinobu Mita
@ 2019-06-02 22:49 ` Chaitanya Kulkarni
0 siblings, 0 replies; 10+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-02 22:49 UTC (permalink / raw)
Thanks for updating the file with nice example, it is really helpful.
Looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 6/2/19 6:51 AM, Akinobu Mita wrote:
> This adds an example of how to inject errors into admin commands.
>
> Cc: Thomas Tai <thomas.tai at oracle.com>
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Jens Axboe <axboe at fb.com>
> Cc: Christoph Hellwig <hch at lst.de>
> Cc: Sagi Grimberg <sagi at grimberg.me>
> Cc: Minwoo Im <minwoo.im.dev at gmail.com>
> Suggested-by: Thomas Tai <thomas.tai at oracle.com>
> Signed-off-by: Akinobu Mita <akinobu.mita at gmail.com>
> ---
> * v2
> - New patch from v2
>
> .../fault-injection/nvme-fault-injection.txt | 56 ++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/Documentation/fault-injection/nvme-fault-injection.txt b/Documentation/fault-injection/nvme-fault-injection.txt
> index 8fbf3bf..efcb339 100644
> --- a/Documentation/fault-injection/nvme-fault-injection.txt
> +++ b/Documentation/fault-injection/nvme-fault-injection.txt
> @@ -114,3 +114,59 @@ R13: ffff88011a3c9680 R14: 0000000000000000 R15: 0000000000000000
> cpu_startup_entry+0x6f/0x80
> start_secondary+0x187/0x1e0
> secondary_startup_64+0xa5/0xb0
> +
> +Example 3: Inject an error into the 10th admin command
> +------------------------------------------------------
> +
> +echo 100 > /sys/kernel/debug/nvme0/fault_inject/probability
> +echo 10 > /sys/kernel/debug/nvme0/fault_inject/space
> +echo 1 > /sys/kernel/debug/nvme0/fault_inject/times
> +nvme reset /dev/nvme0
> +
> +Expected Result:
> +
> +After NVMe controller reset, the reinitialization may or may not succeed.
> +It depends on which admin command is actually forced to fail.
> +
> +Message from dmesg:
> +
> +nvme nvme0: resetting controller
> +FAULT_INJECTION: forcing a failure.
> +name fault_inject, interval 1, probability 100, space 1, times 1
> +CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0-rc2+ #2
> +Hardware name: MSI MS-7A45/B150M MORTAR ARCTIC (MS-7A45), BIOS 1.50 04/25/2017
> +Call Trace:
> + <IRQ>
> + dump_stack+0x63/0x85
> + should_fail+0x14a/0x170
> + nvme_should_fail+0x38/0x80 [nvme_core]
> + nvme_irq+0x129/0x280 [nvme]
> + ? blk_mq_end_request+0xb3/0x120
> + __handle_irq_event_percpu+0x84/0x1a0
> + handle_irq_event_percpu+0x32/0x80
> + handle_irq_event+0x3b/0x60
> + handle_edge_irq+0x7f/0x1a0
> + handle_irq+0x20/0x30
> + do_IRQ+0x4e/0xe0
> + common_interrupt+0xf/0xf
> + </IRQ>
> +RIP: 0010:cpuidle_enter_state+0xc5/0x460
> +Code: ff e8 8f 5f 86 ff 80 7d c7 00 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 69 03 00 00 31 ff e8 62 aa 8c ff fb 66 0f 1f 44 00 00 <45> 85 ed 0f 88 37 03 00 00 4c 8b 45 d0 4c 2b 45 b8 48 ba cf f7 53
> +RSP: 0018:ffffffff88c03dd0 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffdc
> +RAX: ffff9dac25a2ac80 RBX: ffffffff88d53760 RCX: 000000000000001f
> +RDX: 0000000000000000 RSI: 000000002d958403 RDI: 0000000000000000
> +RBP: ffffffff88c03e18 R08: fffffff75e35ffb7 R09: 00000a49a56c0b48
> +R10: ffffffff88c03da0 R11: 0000000000001b0c R12: ffff9dac25a34d00
> +R13: 0000000000000006 R14: 0000000000000006 R15: ffffffff88d53760
> + cpuidle_enter+0x2e/0x40
> + call_cpuidle+0x23/0x40
> + do_idle+0x201/0x280
> + cpu_startup_entry+0x1d/0x20
> + rest_init+0xaa/0xb0
> + arch_call_rest_init+0xe/0x1b
> + start_kernel+0x51c/0x53b
> + x86_64_start_reservations+0x24/0x26
> + x86_64_start_kernel+0x74/0x77
> + secondary_startup_64+0xa4/0xb0
> +nvme nvme0: Could not set queue count (16385)
> +nvme nvme0: IO queues not created
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] Documentation: nvme: add an example for nvme fault injection
[not found] ` <CGME20190602135117epcas4p14feefbe2556a469008a7a499174e5b43@epcms2p1>
@ 2019-06-03 4:37 ` Minwoo Im
0 siblings, 0 replies; 10+ messages in thread
From: Minwoo Im @ 2019-06-03 4:37 UTC (permalink / raw)
This looks good to me.
Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] nvme: enable to inject errors into admin commands
2019-06-02 13:50 ` [PATCH v2 2/3] nvme: enable to inject errors " Akinobu Mita
2019-06-02 22:49 ` Chaitanya Kulkarni
@ 2019-06-04 7:23 ` Christoph Hellwig
2019-06-04 13:36 ` Akinobu Mita
1 sibling, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2019-06-04 7:23 UTC (permalink / raw)
> struct nvme_fault_inject *fault_inject = NULL;
> u16 status;
>
> - /*
> - * make sure this request is coming from a valid namespace
> - */
> if (disk) {
> struct nvme_ns *ns = disk->private_data;
>
> if (ns)
> fault_inject = &ns->fault_inject;
Not new in this code, but how could ns ever be NULL here?
> @@ -257,6 +266,8 @@ struct nvme_ctrl {
> */
> struct thermal_zone_device *tzdev[9];
> #endif
Looks like this series seem to be based on the thermal zone series.
> -struct nvme_fault_inject {
> -#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> - struct fault_attr attr;
> - struct dentry *parent;
> - bool dont_retry; /* DNR, do not retry */
> - u16 status; /* status code */
> -#endif
It would be nicer to already move it up earlier when you change the ifdef
guared.
Otherwise the whole series looks fine:
Reviewed-by: Christoph Hellwig <hch at lst.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] nvme: enable to inject errors into admin commands
2019-06-04 7:23 ` Christoph Hellwig
@ 2019-06-04 13:36 ` Akinobu Mita
0 siblings, 0 replies; 10+ messages in thread
From: Akinobu Mita @ 2019-06-04 13:36 UTC (permalink / raw)
2019?6?4?(?) 16:24 Christoph Hellwig <hch at lst.de>:
>
> > struct nvme_fault_inject *fault_inject = NULL;
> > u16 status;
> >
> > - /*
> > - * make sure this request is coming from a valid namespace
> > - */
> > if (disk) {
> > struct nvme_ns *ns = disk->private_data;
> >
> > if (ns)
> > fault_inject = &ns->fault_inject;
>
>
> Not new in this code, but how could ns ever be NULL here?
Not likely. Should we simply remove this check or add WARN_ON_ONCE()?
> > @@ -257,6 +266,8 @@ struct nvme_ctrl {
> > */
> > struct thermal_zone_device *tzdev[9];
> > #endif
>
> Looks like this series seem to be based on the thermal zone series.
Oops.
> > -struct nvme_fault_inject {
> > -#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> > - struct fault_attr attr;
> > - struct dentry *parent;
> > - bool dont_retry; /* DNR, do not retry */
> > - u16 status; /* status code */
> > -#endif
>
> It would be nicer to already move it up earlier when you change the ifdef
> guared.
OK.
> Otherwise the whole series looks fine:
>
> Reviewed-by: Christoph Hellwig <hch at lst.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-06-04 13:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-02 13:50 [PATCH v2 0/3] nvme: enable to inject errors into admin commands Akinobu Mita
2019-06-02 13:50 ` [PATCH v2 1/3] nvme: prepare for fault injection " Akinobu Mita
2019-06-02 22:49 ` Chaitanya Kulkarni
2019-06-02 13:50 ` [PATCH v2 2/3] nvme: enable to inject errors " Akinobu Mita
2019-06-02 22:49 ` Chaitanya Kulkarni
2019-06-04 7:23 ` Christoph Hellwig
2019-06-04 13:36 ` Akinobu Mita
2019-06-02 13:50 ` [PATCH v2 3/3] Documentation: nvme: add an example for nvme fault injection Akinobu Mita
2019-06-02 22:49 ` Chaitanya Kulkarni
[not found] ` <CGME20190602135117epcas4p14feefbe2556a469008a7a499174e5b43@epcms2p1>
2019-06-03 4:37 ` Minwoo Im
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.