configurable block error injection

configurable block error injection

[Christoph Hellwig]

Hi all,

this series adds a new configurable block error injection facility.
We already have a few to inject block errors, but unfortunately most
of them are either not very useful or hard to use, or both:

 - The fail_make_request failure injection point can't distinguish
   different commands, different ranges in the file and can only injection
   plain I/O errors.
 - the should_fail_bio 'dynamic' failure injection has all the same issues
   as fail_make_request
 - dm-error can only fail all command in the table using BLK_STS_IOERR
   and requires setting up a new block device
 - dm-flakey and dm-dust allow all kinds of configurability, but still
   don't have good error selection, no good support for non-read/write
   commands and are limited to the dm table alignment requirements,
   which for zoned devices enforces setting them up for an entire zone.
   They also once again require setting up a stacked block device,
   which is really annoying in harnesses like xfstests

This series adds a new debugfs-based block layer error injection
that allows to configure what operations and ranges the injection
applied to, and what status to return.  It also allows to configure a
failure ratio similar to the xfs errortag injection.

As part of this the should_fail_bio is removed, as the should_fail_bio
function really gets in the way in it's current form, and the injection
of an errno which always gets turned into BLK_STS_IOERR doesn't make
much sense.  This required adjusting the bpf test suite as it attached
to this function as it assumed it to be present.

Diffstat:
 Documentation/block/error-injection.rst                    |   59 ++
 Documentation/block/index.rst                              |    1 
 block/Kconfig                                              |    6 
 block/Makefile                                             |    1 
 block/blk-core.c                                           |  128 ++---
 block/blk-merge.c                                          |    5 
 block/blk-mq.c                                             |    3 
 block/blk-sysfs.c                                          |    4 
 block/blk.h                                                |   24 
 block/error-injection.c                                    |  329 +++++++++++++
 block/genhd.c                                              |    4 
 include/linux/blk_types.h                                  |    2 
 include/linux/blkdev.h                                     |    5 
 lib/Kconfig.debug                                          |    6 
 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c |    7 
 15 files changed, 507 insertions(+), 77 deletions(-)

[PATCH 1/9] block: remove ALLOW_ERROR_INJECTION for should_fail_bio

[Christoph Hellwig]

Allow error injection for should_fail_bio is a bit misguided.  It allows
inserting an errno, which is then ignored, but it forced and out of line
call for something that should not exist when error injection is disabled.

Remove the error injection flag in preparation for adding better block
layer error injection, and switch the bpf test to use a MM error
injection site instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c                                           | 1 -
 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 7 ++++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index b0f0a304ea0b..3a23af3e26a9 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -545,7 +545,6 @@ int should_fail_bio(struct bio *bio)
 		return -EIO;
 	return 0;
 }
-ALLOW_ERROR_INJECTION(should_fail_bio, ERRNO);
 
 /*
  * Check whether this bio extends beyond the end of the device or partition.
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 2e0ddef77ba5..6c8b161cdd7b 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -588,12 +588,13 @@ static void test_attach_override(void)
 		goto cleanup;
 	}
 
-	/* The should_fail_bio function is on error injection list,
+	/* The __filemap_add_folio function is on error injection list,
 	 * attach should succeed.
 	 */
 	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_override,
-						     "should_fail_bio", NULL);
-	if (!ASSERT_OK_PTR(link, "override_attached_should_fail_bio"))
+						     "__filemap_add_folio,",
+						     NULL);
+	if (!ASSERT_OK_PTR(link, "override_attached___filemap_add_folio,"))
 		goto cleanup;
 
 	bpf_link__destroy(link);
-- 
2.53.0

[PATCH 2/9] block: consolidate the calls to should_fail_bio

[Christoph Hellwig]

Delay the point of error injection a bit so that we have a single site
in blk-core.c after more of the submission side checks and blkcg
throttling.  This allows to make should_fail_bio static in blk-core.c.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c  | 6 +++++-
 block/blk-merge.c | 5 +----
 block/blk.h       | 1 -
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 3a23af3e26a9..f35e0d3fb127 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -539,7 +539,7 @@ static inline void bio_check_ro(struct bio *bio)
 	}
 }
 
-int should_fail_bio(struct bio *bio)
+static int should_fail_bio(struct bio *bio)
 {
 	if (should_fail_request(bdev_whole(bio->bi_bdev), bio->bi_iter.bi_size))
 		return -EIO;
@@ -723,6 +723,10 @@ static void __submit_bio_noacct_mq(struct bio *bio)
 
 void submit_bio_noacct_nocheck(struct bio *bio, bool split)
 {
+	if (should_fail_bio(bio)) {
+		bio_io_error(bio);
+		return;
+	}
 	blk_cgroup_bio_start(bio);
 
 	if (!bio_flagged(bio, BIO_TRACE_COMPLETION)) {
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 7cc82a7a6f4e..b44f8ae849b8 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -130,11 +130,8 @@ struct bio *bio_submit_split_bioset(struct bio *bio, unsigned int split_sectors,
 	trace_block_split(split, bio->bi_iter.bi_sector);
 	WARN_ON_ONCE(bio_zone_write_plugging(bio));
 
-	if (should_fail_bio(bio))
-		bio_io_error(bio);
-	else if (!blk_throtl_bio(bio))
+	if (!blk_throtl_bio(bio))
 		submit_bio_noacct_nocheck(bio, true);
-
 	return split;
 }
 EXPORT_SYMBOL_GPL(bio_submit_split_bioset);
diff --git a/block/blk.h b/block/blk.h
index bf1a80493ff1..889a39589356 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -646,7 +646,6 @@ extern const struct address_space_operations def_blk_aops;
 int disk_register_independent_access_ranges(struct gendisk *disk);
 void disk_unregister_independent_access_ranges(struct gendisk *disk);
 
-int should_fail_bio(struct bio *bio);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 bool should_fail_request(struct block_device *part, unsigned int bytes);
 #else /* CONFIG_FAIL_MAKE_REQUEST */
-- 
2.53.0

[PATCH 3/9] block: refactor should_fail_bio and should_fail_request

[Christoph Hellwig]

Move the bdev flag checks into a helper and the blk-mq clone
insers so that we can do only a single actual error injection for
I/O to partitions instead of doing it twice.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c          | 28 ++++++++++++++--------------
 block/blk-mq.c            |  3 ++-
 block/blk.h               |  5 ++---
 include/linux/blk_types.h |  2 --
 4 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index f35e0d3fb127..644888b66f33 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -502,10 +502,9 @@ static int __init setup_fail_make_request(char *str)
 }
 __setup("fail_make_request=", setup_fail_make_request);
 
-bool should_fail_request(struct block_device *part, unsigned int bytes)
+bool should_fail_request(unsigned int bytes)
 {
-	return bdev_test_flag(part, BD_MAKE_IT_FAIL) &&
-	       should_fail(&fail_make_request, bytes);
+	return should_fail(&fail_make_request, bytes);
 }
 
 static int __init fail_make_request_debugfs(void)
@@ -539,11 +538,13 @@ static inline void bio_check_ro(struct bio *bio)
 	}
 }
 
-static int should_fail_bio(struct bio *bio)
+static inline bool may_fail_bio(struct bio *bio)
 {
-	if (should_fail_request(bdev_whole(bio->bi_bdev), bio->bi_iter.bi_size))
-		return -EIO;
-	return 0;
+	if (!IS_ENABLED(CONFIG_FAIL_MAKE_REQUEST))
+		return false;
+	return bdev_test_flag(bio->bi_bdev, BD_MAKE_IT_FAIL) ||
+		(bio_flagged(bio, BIO_REMAPPED) &&
+		 bdev_test_flag(bdev_whole(bio->bi_bdev), BD_MAKE_IT_FAIL));
 }
 
 /*
@@ -577,8 +578,6 @@ static int blk_partition_remap(struct bio *bio)
 {
 	struct block_device *p = bio->bi_bdev;
 
-	if (unlikely(should_fail_request(p, bio->bi_iter.bi_size)))
-		return -EIO;
 	if (bio_sectors(bio)) {
 		bio->bi_iter.bi_sector += p->bd_start_sect;
 		trace_block_bio_remap(bio, p->bd_dev,
@@ -723,10 +722,13 @@ static void __submit_bio_noacct_mq(struct bio *bio)
 
 void submit_bio_noacct_nocheck(struct bio *bio, bool split)
 {
-	if (should_fail_bio(bio)) {
-		bio_io_error(bio);
-		return;
+	if (unlikely(may_fail_bio(bio))) {
+		if (should_fail_request(bio->bi_iter.bi_size)) {
+			bio_io_error(bio);
+			return;
+		}
 	}
+
 	blk_cgroup_bio_start(bio);
 
 	if (!bio_flagged(bio, BIO_TRACE_COMPLETION)) {
@@ -799,8 +801,6 @@ void submit_bio_noacct(struct bio *bio)
 			goto not_supported;
 	}
 
-	if (should_fail_bio(bio))
-		goto end_io;
 	bio_check_ro(bio);
 	if (!bio_flagged(bio, BIO_REMAPPED)) {
 		if (unlikely(bio_check_eod(bio)))
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 629e16003eb7..bf66645622df 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3275,7 +3275,8 @@ blk_status_t blk_insert_cloned_request(struct request *rq)
 		return BLK_STS_IOERR;
 	}
 
-	if (q->disk && should_fail_request(q->disk->part0, blk_rq_bytes(rq)))
+	if (q->disk && bdev_test_flag(q->disk->part0, BD_MAKE_IT_FAIL) &&
+	    should_fail_request(blk_rq_bytes(rq)))
 		return BLK_STS_IOERR;
 
 	ret = blk_crypto_rq_get_keyslot(rq);
diff --git a/block/blk.h b/block/blk.h
index 889a39589356..250a6eee700a 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -647,10 +647,9 @@ int disk_register_independent_access_ranges(struct gendisk *disk);
 void disk_unregister_independent_access_ranges(struct gendisk *disk);
 
 #ifdef CONFIG_FAIL_MAKE_REQUEST
-bool should_fail_request(struct block_device *part, unsigned int bytes);
+bool should_fail_request(unsigned int bytes);
 #else /* CONFIG_FAIL_MAKE_REQUEST */
-static inline bool should_fail_request(struct block_device *part,
-					unsigned int bytes)
+static inline bool should_fail_request(unsigned int bytes)
 {
 	return false;
 }
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 8808ee76e73c..4a3cfa857637 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -51,9 +51,7 @@ struct block_device {
 #define BD_WRITE_HOLDER		(1u<<9)
 #define BD_HAS_SUBMIT_BIO	(1u<<10)
 #define BD_RO_WARNED		(1u<<11)
-#ifdef CONFIG_FAIL_MAKE_REQUEST
 #define BD_MAKE_IT_FAIL		(1u<<12)
-#endif
 	dev_t			bd_dev;
 	struct address_space	*bd_mapping;	/* page cache */
 
-- 
2.53.0

[PATCH 4/9] block: move the FAIL_MAKE_REQUEST symbol from lib/ to block/

[Christoph Hellwig]

Keep the Kconfig symbol together with the code that it guards.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/Kconfig     | 6 ++++++
 lib/Kconfig.debug | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/Kconfig b/block/Kconfig
index 15027963472d..6c942391f65e 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -209,6 +209,12 @@ config BLK_INLINE_ENCRYPTION_FALLBACK
 	  by falling back to the kernel crypto API when inline
 	  encryption hardware is not present.
 
+config FAIL_MAKE_REQUEST
+	bool "Fault-injection capability for disk IO"
+	depends on FAULT_INJECTION
+	help
+	  Provide fault-injection capability for disk IO.
+
 source "block/partitions/Kconfig"
 
 config BLK_PM
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8ff5adcfe1e0..fb085963ec5e 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2116,12 +2116,6 @@ config FAULT_INJECTION_USERCOPY
 	  Provides fault-injection capability to inject failures
 	  in usercopy functions (copy_from_user(), get_user(), ...).
 
-config FAIL_MAKE_REQUEST
-	bool "Fault-injection capability for disk IO"
-	depends on FAULT_INJECTION && BLOCK
-	help
-	  Provide fault-injection capability for disk IO.
-
 config FAIL_IO_TIMEOUT
 	bool "Fault-injection capability for faking disk interrupts"
 	depends on FAULT_INJECTION && BLOCK
-- 
2.53.0

[PATCH 5/9] block: add a macro to initialize the status table

[Christoph Hellwig]

Prepare for adding a new value to the error table by adding a macro
to fill it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 644888b66f33..1ab666fc2e27 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -132,39 +132,44 @@ inline const char *blk_op_str(enum req_op op)
 }
 EXPORT_SYMBOL_GPL(blk_op_str);
 
+#define ENT(_tag, _errno, _desc)	\
+[BLK_STS_##_tag] = {				\
+	.errno		= _errno,		\
+	.name		= _desc,		\
+}
 static const struct {
 	int		errno;
 	const char	*name;
 } blk_errors[] = {
-	[BLK_STS_OK]		= { 0,		"" },
-	[BLK_STS_NOTSUPP]	= { -EOPNOTSUPP, "operation not supported" },
-	[BLK_STS_TIMEOUT]	= { -ETIMEDOUT,	"timeout" },
-	[BLK_STS_NOSPC]		= { -ENOSPC,	"critical space allocation" },
-	[BLK_STS_TRANSPORT]	= { -ENOLINK,	"recoverable transport" },
-	[BLK_STS_TARGET]	= { -EREMOTEIO,	"critical target" },
-	[BLK_STS_RESV_CONFLICT]	= { -EBADE,	"reservation conflict" },
-	[BLK_STS_MEDIUM]	= { -ENODATA,	"critical medium" },
-	[BLK_STS_PROTECTION]	= { -EILSEQ,	"protection" },
-	[BLK_STS_RESOURCE]	= { -ENOMEM,	"kernel resource" },
-	[BLK_STS_DEV_RESOURCE]	= { -EBUSY,	"device resource" },
-	[BLK_STS_AGAIN]		= { -EAGAIN,	"nonblocking retry" },
-	[BLK_STS_OFFLINE]	= { -ENODEV,	"device offline" },
+	ENT(OK,			0,		""),
+	ENT(NOTSUPP,		-EOPNOTSUPP,	"operation not supported"),
+	ENT(TIMEOUT,		-ETIMEDOUT,	"timeout"),
+	ENT(NOSPC,		-ENOSPC,	"critical space allocation"),
+	ENT(TRANSPORT,		-ENOLINK,	"recoverable transport"),
+	ENT(TARGET,		-EREMOTEIO,	"critical target"),
+	ENT(RESV_CONFLICT,	-EBADE,		"reservation conflict"),
+	ENT(MEDIUM,		-ENODATA,	"critical medium"),
+	ENT(PROTECTION,		-EILSEQ,	"protection"),
+	ENT(RESOURCE,		-ENOMEM,	"kernel resource"),
+	ENT(DEV_RESOURCE,	-EBUSY,		"device resource"),
+	ENT(AGAIN,		-EAGAIN,	"nonblocking retry"),
+	ENT(OFFLINE,		-ENODEV,	"device offline"),
 
 	/* device mapper special case, should not leak out: */
-	[BLK_STS_DM_REQUEUE]	= { -EREMCHG, "dm internal retry" },
+	ENT(DM_REQUEUE,		-EREMCHG,	"dm internal retry"),
 
 	/* zone device specific errors */
-	[BLK_STS_ZONE_OPEN_RESOURCE]	= { -ETOOMANYREFS, "open zones exceeded" },
-	[BLK_STS_ZONE_ACTIVE_RESOURCE]	= { -EOVERFLOW, "active zones exceeded" },
+	ENT(ZONE_OPEN_RESOURCE, -ETOOMANYREFS,	"open zones exceeded"),
+	ENT(ZONE_ACTIVE_RESOURCE, -EOVERFLOW,	"active zones exceeded"),
 
 	/* Command duration limit device-side timeout */
-	[BLK_STS_DURATION_LIMIT]	= { -ETIME, "duration limit exceeded" },
-
-	[BLK_STS_INVAL]		= { -EINVAL,	"invalid" },
+	ENT(DURATION_LIMIT,	-ETIME,		"duration limit exceeded"),
+	ENT(INVAL,		-EINVAL,	"invalid"),
 
 	/* everything else not covered above: */
-	[BLK_STS_IOERR]		= { -EIO,	"I/O" },
+	ENT(IOERR,		-EIO,		"I/O"),
 };
+#undef ENT
 
 blk_status_t errno_to_blk_status(int errno)
 {
-- 
2.53.0

[PATCH 6/9] block: add a "tag" for block status codes

[Christoph Hellwig]

The full name of the status codes is not good for user interfaces as it
can contain white spaces.  Add the name of the status code without the
BLK_STS_ prefix as a tag so that it can be used for user interfaces.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c | 24 ++++++++++++++++++++++++
 block/blk.h      |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1ab666fc2e27..19a4d0672b3d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -135,10 +135,12 @@ EXPORT_SYMBOL_GPL(blk_op_str);
 #define ENT(_tag, _errno, _desc)	\
 [BLK_STS_##_tag] = {				\
 	.errno		= _errno,		\
+	.tag		= __stringify(_tag),	\
 	.name		= _desc,		\
 }
 static const struct {
 	int		errno;
+	const char	*tag;
 	const char	*name;
 } blk_errors[] = {
 	ENT(OK,			0,		""),
@@ -203,6 +205,28 @@ const char *blk_status_to_str(blk_status_t status)
 	return blk_errors[idx].name;
 }
 
+const char *blk_status_to_tag(blk_status_t status)
+{
+	int idx = (__force int)status;
+
+	if (WARN_ON_ONCE(idx >= ARRAY_SIZE(blk_errors)))
+		return "<null>";
+	return blk_errors[idx].tag;
+}
+
+blk_status_t tag_to_blk_status(const char *tag)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(blk_errors); i++) {
+		if (blk_errors[i].tag &&
+		    !strcmp(blk_errors[i].tag, tag))
+			return (__force blk_status_t)i;
+	}
+
+	return BLK_STS_OK;
+}
+
 /**
  * blk_sync_queue - cancel any pending callbacks on a queue
  * @q: the queue
diff --git a/block/blk.h b/block/blk.h
index 250a6eee700a..1e80338af858 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -50,6 +50,8 @@ struct blk_flush_queue *blk_alloc_flush_queue(int node, int cmd_size,
 void blk_free_flush_queue(struct blk_flush_queue *q);
 
 const char *blk_status_to_str(blk_status_t status);
+const char *blk_status_to_tag(blk_status_t status);
+blk_status_t tag_to_blk_status(const char *tag);
 
 bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
 bool blk_queue_start_drain(struct request_queue *q);
-- 
2.53.0

[PATCH 7/9] block: add a str_to_blk_op helper

[Christoph Hellwig]

Add a helper to find the REQ_OP_XYZ constant from the "XYZ" string.
This will be used for the error injection debugfs interface.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c | 13 +++++++++++++
 block/blk.h      |  1 +
 2 files changed, 14 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 19a4d0672b3d..8bbc03ce924f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -132,6 +132,19 @@ inline const char *blk_op_str(enum req_op op)
 }
 EXPORT_SYMBOL_GPL(blk_op_str);
 
+enum req_op str_to_blk_op(const char *op)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(blk_op_name); i++) {
+		if (blk_op_name[i] &&
+		    !strcmp(blk_op_name[i], op))
+			return i;
+	}
+
+	return REQ_OP_LAST;
+}
+
 #define ENT(_tag, _errno, _desc)	\
 [BLK_STS_##_tag] = {				\
 	.errno		= _errno,		\
diff --git a/block/blk.h b/block/blk.h
index 1e80338af858..4857b899e2b6 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -52,6 +52,7 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
 const char *blk_status_to_str(blk_status_t status);
 const char *blk_status_to_tag(blk_status_t status);
 blk_status_t tag_to_blk_status(const char *tag);
+enum req_op str_to_blk_op(const char *op);
 
 bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
 bool blk_queue_start_drain(struct request_queue *q);
-- 
2.53.0

[PATCH 8/9] block: add configurable error injection

[Christoph Hellwig]

Add a new block error injection interface that allows to inject specific
status code for specific ranges.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/block/error-injection.rst |  59 +++++
 Documentation/block/index.rst           |   1 +
 block/Makefile                          |   1 +
 block/blk-core.c                        |   2 +
 block/blk-sysfs.c                       |   4 +
 block/blk.h                             |  15 ++
 block/error-injection.c                 | 299 ++++++++++++++++++++++++
 block/genhd.c                           |   4 +
 include/linux/blkdev.h                  |   5 +
 9 files changed, 390 insertions(+)
 create mode 100644 Documentation/block/error-injection.rst
 create mode 100644 block/error-injection.c

diff --git a/Documentation/block/error-injection.rst b/Documentation/block/error-injection.rst
new file mode 100644
index 000000000000..be87091b5330
--- /dev/null
+++ b/Documentation/block/error-injection.rst
@@ -0,0 +1,59 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================
+Configurable Error Injection
+============================
+
+Overview
+--------
+
+Configurable error injection allows injecting specific block layer status codes
+for ranges of a block device.  Error can be injected unconditional, or with a
+given probability.
+
+To use configurable error injection, CONFIG_FAIL_MAKE_REQUEST must be enabled.
+
+The only interface is the error_injection debugfs file, which is created for
+each registered gendisk.  Writes to this file are used to create or delete rules
+and reads return a list of the current error injection sites.
+
+Options
+-------
+
+The following options specify the operations:
+
+===================	=======================================================
+add			add a new rule
+removeall		remove all existing rules
+===================	=======================================================
+
+The following options specify the details of the rule for the add operation:
+
+===================	=======================================================
+op=%s			block layer operation this rule applies to, e.g. READ
+			or WRITE.
+			Mandatory.
+start=%u		First block layer sector the rule applies to.
+			Optional, defaults to 0.
+nr_sectors=%u		Number of sectors this rule applies.
+			Optional, defaults to the remainder of the device.
+status=%s		Status to return.
+			Mandatory.
+chance=%u		Only return a failure with a likelihood of 1/chance.
+			Optional, defaults to 1 (always).
+===================	=======================================================
+
+Example
+-------
+
+Return BLK_STS_IOERR for one in 10 reads of sector 0 of /dev/nvme0n1:
+
+	$ echo 'add,op=READ,start=0,status=IOERR,chance=10' > /sys/kernel/debug/block/nvme0n1/error_injection
+
+Return BLK_STS_MEDIUM for every write to /dev/nvme0n1:
+
+	$ echo 'add,op=WRITE,start=0,status=MEDIUM' > /sys/kernel/debug/block/nvme0n1/error_injection
+
+Remove all rules for /dev/nvme0n1:
+
+	$ echo 'removeall' > /sys/kernel/debug/block/nvme0n1/error_injection
diff --git a/Documentation/block/index.rst b/Documentation/block/index.rst
index 9fea696f9daa..bfa1bbd31ddf 100644
--- a/Documentation/block/index.rst
+++ b/Documentation/block/index.rst
@@ -22,3 +22,4 @@ Block
    switching-sched
    writeback_cache_control
    ublk
+   error-injection
diff --git a/block/Makefile b/block/Makefile
index 7dce2e44276c..d223b6b7d72f 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -11,6 +11,7 @@ obj-y		:= bdev.o fops.o bio.o elevator.o blk-core.o blk-sysfs.o \
 			genhd.o ioprio.o badblocks.o partitions/ blk-rq-qos.o \
 			disk-events.o blk-ia-ranges.o early-lookup.o
 
+obj-$(CONFIG_FAIL_MAKE_REQUEST)	+= error-injection.o
 obj-$(CONFIG_BLK_DEV_BSG_COMMON) += bsg.o
 obj-$(CONFIG_BLK_DEV_BSGLIB)	+= bsg-lib.o
 obj-$(CONFIG_BLK_CGROUP)	+= blk-cgroup.o
diff --git a/block/blk-core.c b/block/blk-core.c
index 8bbc03ce924f..04a392849ab0 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -765,6 +765,8 @@ static void __submit_bio_noacct_mq(struct bio *bio)
 void submit_bio_noacct_nocheck(struct bio *bio, bool split)
 {
 	if (unlikely(may_fail_bio(bio))) {
+		if (blk_error_inject(bio))
+			return;
 		if (should_fail_request(bio->bi_iter.bi_size)) {
 			bio_io_error(bio);
 			return;
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index f22c1f253eb3..43f909c7f0c9 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -933,6 +933,8 @@ static void blk_debugfs_remove(struct gendisk *disk)
 
 	blk_debugfs_lock_nomemsave(q);
 	blk_trace_shutdown(q);
+	if (IS_ENABLED(CONFIG_FAIL_MAKE_REQUEST))
+		blk_error_injection_exit(disk);
 	debugfs_remove_recursive(q->debugfs_dir);
 	q->debugfs_dir = NULL;
 	q->sched_debugfs_dir = NULL;
@@ -963,6 +965,8 @@ int blk_register_queue(struct gendisk *disk)
 
 	memflags = blk_debugfs_lock(q);
 	q->debugfs_dir = debugfs_create_dir(disk->disk_name, blk_debugfs_root);
+	if (IS_ENABLED(CONFIG_FAIL_MAKE_REQUEST))
+		blk_error_injection_init(disk);
 	if (queue_is_mq(q))
 		blk_mq_debugfs_register(q);
 	blk_debugfs_unlock(q, memflags);
diff --git a/block/blk.h b/block/blk.h
index 4857b899e2b6..19f925d8f39d 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -781,4 +781,19 @@ static inline void blk_debugfs_unlock(struct request_queue *q,
 	memalloc_noio_restore(memflags);
 }
 
+void blk_error_injection_init(struct gendisk *disk);
+void blk_error_injection_exit(struct gendisk *disk);
+
+bool __blk_error_inject(struct bio *bio);
+static inline bool blk_error_inject(struct bio *bio)
+{
+#ifdef CONFIG_FAIL_MAKE_REQUEST
+	struct gendisk *disk = bio->bi_bdev->bd_disk;
+
+	if (!list_empty_careful(&disk->error_injection_list))
+		return __blk_error_inject(bio);
+#endif
+	return false;
+}
+
 #endif /* BLK_INTERNAL_H */
diff --git a/block/error-injection.c b/block/error-injection.c
new file mode 100644
index 000000000000..dc0420c4eb58
--- /dev/null
+++ b/block/error-injection.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 Christoph Hellwig.
+ */
+#include <linux/debugfs.h>
+#include <linux/blkdev.h>
+#include <linux/parser.h>
+#include <linux/seq_file.h>
+#include "blk.h"
+
+struct blk_error_inject {
+	struct list_head		entry;
+	sector_t			start;
+	sector_t			end;
+	enum req_op			op;
+	blk_status_t			status;
+
+	/* only inject every 1 / chance times */
+	unsigned int			chance;
+};
+
+bool __blk_error_inject(struct bio *bio)
+{
+	struct gendisk *disk = bio->bi_bdev->bd_disk;
+	struct blk_error_inject *inj;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(inj, &disk->error_injection_list, entry) {
+		if (bio->bi_iter.bi_sector <= inj->end &&
+		    bio_end_sector(bio) >= inj->start &&
+		    bio_op(bio) == inj->op) {
+			blk_status_t status = inj->status;
+
+			if (inj->chance > 1 &&
+			    (get_random_u32() % inj->chance) != 0)
+				continue;
+
+			rcu_read_unlock();
+			pr_info_ratelimited("%pg: injecting %s error for %s at sector %llu:%u\n",
+					disk->part0,
+					blk_status_to_str(status),
+					blk_op_str(inj->op),
+					bio->bi_iter.bi_sector,
+					bio_sectors(bio));
+			bio_endio_status(bio, status);
+			return true;
+		}
+	}
+	rcu_read_unlock();
+	return false;
+}
+
+static int error_inject_add(struct gendisk *disk, enum req_op op,
+		sector_t start, u64 nr_sectors, blk_status_t status,
+		unsigned int chance)
+{
+	struct blk_error_inject *inj;
+
+	if (op == REQ_OP_LAST)
+		return -EINVAL;
+	if (status == BLK_STS_OK)
+		return -EINVAL;
+	if (U64_MAX - nr_sectors < start)
+		return -EINVAL;
+
+	if (!nr_sectors)
+		nr_sectors = U64_MAX;
+
+	inj = kzalloc_obj(*inj);
+	if (!inj)
+		return -ENOMEM;
+
+	pr_debug_ratelimited("%pg: adding %s injection for %s at sector %llu:%llu\n",
+			disk->part0, blk_status_to_str(status),
+			blk_op_str(op),
+			start, nr_sectors);
+
+	inj->op = op;
+	inj->start = start;
+	inj->end = start + nr_sectors - 1;
+	inj->status = status;
+	inj->chance = chance;
+
+	/*
+	 * Add to the front of the list so that newer entries can partially
+	 * override other entries.  This also intentional allows duplicate
+	 * entries as there is no real reason to reject them.
+	 */
+	mutex_lock(&disk->error_injection_lock);
+	if (!disk_live(disk)) {
+		mutex_unlock(&disk->error_injection_lock);
+		return -EINVAL;
+	}
+	list_add(&inj->entry, &disk->error_injection_list);
+	mutex_unlock(&disk->error_injection_lock);
+
+	bdev_set_flag(disk->part0, BD_MAKE_IT_FAIL);
+	return 0;
+}
+
+static void error_inject_removall(struct gendisk *disk)
+{
+	struct blk_error_inject *inj;
+
+	mutex_lock(&disk->error_injection_lock);
+	while ((inj = list_first_entry_or_null(&disk->error_injection_list,
+			struct blk_error_inject, entry))) {
+		list_del_rcu(&inj->entry);
+		mutex_unlock(&disk->error_injection_lock);
+
+		kfree_rcu_mightsleep(inj);
+
+		mutex_lock(&disk->error_injection_lock);
+	}
+
+	mutex_unlock(&disk->error_injection_lock);
+
+	bdev_clear_flag(disk->part0, BD_MAKE_IT_FAIL);
+}
+
+enum options {
+	Opt_add			= (1u << 0),
+	Opt_removeall		= (1u << 1),
+
+	Opt_op			= (1u << 16),
+	Opt_start		= (1u << 17),
+	Opt_nr_sectors		= (1u << 18),
+	Opt_status		= (1u << 19),
+	Opt_chance		= (1u << 20),
+
+	Opt_invalid,
+};
+
+static const match_table_t opt_tokens = {
+	{ Opt_add,			"add",			},
+	{ Opt_removeall,		"removeall",		},
+	{ Opt_op,			"op=%s",		},
+	{ Opt_start,			"start=%u"		},
+	{ Opt_nr_sectors,		"nr_sectors=%u"		},
+	{ Opt_status,			"status=%s"		},
+	{ Opt_chance,			"chance=%u"		},
+	{ Opt_invalid,			NULL,			},
+};
+
+static int match_op(substring_t *args, enum req_op *op)
+{
+	const char *tag;
+
+	tag = match_strdup(args);
+	if (!tag)
+		return -ENOMEM;
+	*op = str_to_blk_op(tag);
+	if (*op == REQ_OP_LAST)
+		pr_warn("invalid op '%s'\n", tag);
+	kfree(tag);
+	return 0;
+}
+
+static int match_status(substring_t *args, blk_status_t *status)
+{
+	const char *tag;
+
+	tag = match_strdup(args);
+	if (!tag)
+		return -ENOMEM;
+	*status = tag_to_blk_status(tag);
+	if (!*status)
+		pr_warn("invalid status '%s'\n", tag);
+	kfree(tag);
+	return 0;
+}
+
+static ssize_t blk_error_injection_write(struct file *file,
+		const char __user *ubuf, size_t count, loff_t *pos)
+{
+	struct gendisk *disk = file_inode(file)->i_private;
+	enum { Unset, Add, Removeall } action = Unset;
+	unsigned int option_mask = 0, chance = 1;
+	enum req_op op = REQ_OP_LAST;
+	u64 start = 0, nr_sectors = 0;
+	blk_status_t status = BLK_STS_OK;
+	substring_t args[MAX_OPT_ARGS];
+	char *options, *o, *p;
+	ssize_t token, ret = 0;
+
+	options = memdup_user_nul(ubuf, count);
+	if (!options)
+		return -ENOMEM;
+
+	o = options;
+	while ((p = strsep(&o, ",\n")) != NULL) {
+		if (!*p)
+			continue;
+		token = match_token(p, opt_tokens, args);
+		option_mask |= token;
+		switch (token) {
+		case Opt_add:
+			if (action == Unset)
+				action = Add;
+			else
+				ret = -EINVAL;
+			break;
+		case Opt_removeall:
+			if (action == Unset)
+				action = Removeall;
+			else
+				ret = -EINVAL;
+			break;
+		case Opt_op:
+			ret = match_op(args, &op);
+			break;
+		case Opt_start:
+			ret = match_u64(args, &start);
+			break;
+		case Opt_nr_sectors:
+			ret = match_u64(args, &nr_sectors);
+			break;
+		case Opt_status:
+			ret = match_status(args, &status);
+			break;
+		case Opt_chance:
+			ret = match_uint(args, &chance);
+			if (!ret && chance == 0)
+				ret = -EINVAL;
+			break;
+		default:
+			pr_warn("unknown parameter or missing value '%s'\n", p);
+			ret = -EINVAL;
+			break;
+		}
+		if (ret)
+			goto out_free_options;
+	}
+
+	switch (action) {
+	case Add:
+		ret = error_inject_add(disk, op, start, nr_sectors, status,
+				chance);
+		break;
+	case Removeall:
+		if (option_mask & ~Opt_removeall)
+			return -EINVAL;
+		error_inject_removall(disk);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	if (!ret)
+		ret = count;
+out_free_options:
+	kfree(options);
+	return ret;
+}
+
+static int blk_error_injection_show(struct seq_file *s, void *private)
+{
+	struct gendisk *disk = s->private;
+	struct blk_error_inject *inj;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(inj, &disk->error_injection_list, entry) {
+		seq_printf(s, "%llu:%llu status=%s,chance=%u",
+			inj->start, inj->end,
+			blk_status_to_tag(inj->status), inj->chance);
+		seq_putc(s, '\n');
+	}
+	rcu_read_unlock();
+	return 0;
+}
+
+static int blk_error_injection_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, blk_error_injection_show, inode->i_private);
+}
+
+static int blk_error_injection_release(struct inode *inode, struct file *file)
+{
+	return single_release(inode, file);
+}
+
+static const struct file_operations blk_error_injection_fops = {
+	.owner		= THIS_MODULE,
+	.write		= blk_error_injection_write,
+	.read		= seq_read,
+	.open		= blk_error_injection_open,
+	.release	= blk_error_injection_release,
+};
+
+void blk_error_injection_init(struct gendisk *disk)
+{
+	debugfs_create_file("error_injection", 0600, disk->queue->debugfs_dir,
+			disk, &blk_error_injection_fops);
+}
+
+void blk_error_injection_exit(struct gendisk *disk)
+{
+	error_inject_removall(disk);
+}
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e9..30f42461d895 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1485,6 +1485,10 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 	lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
 #ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 	INIT_LIST_HEAD(&disk->slave_bdevs);
+#endif
+#ifdef CONFIG_FAIL_MAKE_REQUEST
+	mutex_init(&disk->error_injection_lock);
+	INIT_LIST_HEAD(&disk->error_injection_list);
 #endif
 	mutex_init(&disk->rqos_state_mutex);
 	kobject_init(&disk->queue_kobj, &blk_queue_ktype);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 17270a28c66d..8743ad616b7f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -227,6 +227,11 @@ struct gendisk {
 	 */
 	struct blk_independent_access_ranges *ia_ranges;
 
+#ifdef CONFIG_FAIL_MAKE_REQUEST
+	struct mutex		error_injection_lock;
+	struct list_head	error_injection_list;
+#endif
+
 	struct mutex rqos_state_mutex;	/* rqos state change mutex */
 };
 
-- 
2.53.0

Re: [PATCH 8/9] block: add configurable error injection

[Keith Busch]

On Tue, Jun 03, 2026 at 07:45:40AM +0200, Christoph Hellwig wrote:
> +static int error_inject_add(struct gendisk *disk, enum req_op op,
> +		sector_t start, u64 nr_sectors, blk_status_t status,
> +		unsigned int chance)
> +{
> +	struct blk_error_inject *inj;
> +
> +	if (op == REQ_OP_LAST)
> +		return -EINVAL;
> +	if (status == BLK_STS_OK)
> +		return -EINVAL;
> +	if (U64_MAX - nr_sectors < start)
> +		return -EINVAL;
> +
> +	if (!nr_sectors)
> +		nr_sectors = U64_MAX;
> +

...

> +
> +	inj->op = op;
> +	inj->start = start;
> +	inj->end = start + nr_sectors - 1;

When nr_sectors is 0, it is reset to U64_MAX so overflows if start > 1.
I think you want to remove overriding nr_sectors to U64_MAX and do:

	if (!nr_sectors)
		inj->end = U64_MAX;
	else if (U64_MAX - nr_sectors < start )
		return -EINVAL;
	else
		inj->end = start + nr_sectors - 1;

> +	inj->status = status;
> +	inj->chance = chance;
> +
> +	/*
> +	 * Add to the front of the list so that newer entries can partially
> +	 * override other entries.  This also intentional allows duplicate
> +	 * entries as there is no real reason to reject them.
> +	 */
> +	mutex_lock(&disk->error_injection_lock);
> +	if (!disk_live(disk)) {
> +		mutex_unlock(&disk->error_injection_lock);
> +		return -EINVAL;

I think we've leaked 'inj' in this error case.

> +	}
> +	list_add(&inj->entry, &disk->error_injection_list);

The __blk_error_inject interates this list with
"list_for_each_entry_rcu", so shouldn't this be list_add_rcu to match?

> +	mutex_unlock(&disk->error_injection_lock);
> +
> +	bdev_set_flag(disk->part0, BD_MAKE_IT_FAIL);
> +	return 0;
> +}

<snip>

> +static const match_table_t opt_tokens = {
> +	{ Opt_add,			"add",			},
> +	{ Opt_removeall,		"removeall",		},
> +	{ Opt_op,			"op=%s",		},
> +	{ Opt_start,			"start=%u"		},
> +	{ Opt_nr_sectors,		"nr_sectors=%u"		},

Shouldn't start and nr_sectors use %llu?

> +static ssize_t blk_error_injection_write(struct file *file,
> +		const char __user *ubuf, size_t count, loff_t *pos)
> +{

...

> +	options = memdup_user_nul(ubuf, count);
> +	if (!options)
> +		return -ENOMEM;
> +

On failure, memdup_user_nul returns an ERR_PTR rather than NULL.

	if (IS_ERR(options))
		return PTR_ERR(options);

> +	case Removeall:
> +		if (option_mask & ~Opt_removeall)
> +			return -EINVAL;

Leaking "options"? Should this be:

		if (option_mask & ~Opt_removeall) {
			ret = -EINVAL;
			goto out_free_options;
		}

?

> +		error_inject_removall(disk);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	if (!ret)
> +		ret = count;
> +out_free_options:
> +	kfree(options);
> +	return ret;
> +}

Re: [PATCH 8/9] block: add configurable error injection

[Christoph Hellwig]

On Tue, Jun 02, 2026 at 10:42:35AM +0100, Keith Busch wrote:
> When nr_sectors is 0, it is reset to U64_MAX so overflows if start > 1.

Yeah.

> I think you want to remove overriding nr_sectors to U64_MAX and do:
> 
> 	if (!nr_sectors)
> 		inj->end = U64_MAX;
> 	else if (U64_MAX - nr_sectors < start )
> 		return -EINVAL;
> 	else
> 		inj->end = start + nr_sectors - 1;

I ended up ordering a bit differently for better readability, but
yes.

> > +	mutex_lock(&disk->error_injection_lock);
> > +	if (!disk_live(disk)) {
> > +		mutex_unlock(&disk->error_injection_lock);
> > +		return -EINVAL;
> 
> I think we've leaked 'inj' in this error case.

Yes.

> 
> > +	}
> > +	list_add(&inj->entry, &disk->error_injection_list);
> 
> The __blk_error_inject interates this list with
> "list_for_each_entry_rcu", so shouldn't this be list_add_rcu to match?

Yes.

> > +static const match_table_t opt_tokens = {
> > +	{ Opt_add,			"add",			},
> > +	{ Opt_removeall,		"removeall",		},
> > +	{ Opt_op,			"op=%s",		},
> > +	{ Opt_start,			"start=%u"		},
> > +	{ Opt_nr_sectors,		"nr_sectors=%u"		},
> 
> Shouldn't start and nr_sectors use %llu?

lib/parser.c doesn't use those prefixes, it's a bit weird.

> > +	if (!options)
> > +		return -ENOMEM;
> > +
> 
> On failure, memdup_user_nul returns an ERR_PTR rather than NULL.
> 
> 	if (IS_ERR(options))
> 		return PTR_ERR(options);

Aarg, annoying.  Because memdup_user does return NULL :(

> 
> > +	case Removeall:
> > +		if (option_mask & ~Opt_removeall)
> > +			return -EINVAL;
> 
> Leaking "options"? Should this be:
> 
> 		if (option_mask & ~Opt_removeall) {
> 			ret = -EINVAL;
> 			goto out_free_options;
> 		}
> 
> ?

Yes.

Re: [PATCH 8/9] block: add configurable error injection

[Randy Dunlap]

On 6/1/26 10:45 PM, Christoph Hellwig wrote:
> diff --git a/Documentation/block/error-injection.rst b/Documentation/block/error-injection.rst
> new file mode 100644
> index 000000000000..be87091b5330
> --- /dev/null
> +++ b/Documentation/block/error-injection.rst
> @@ -0,0 +1,59 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============================
> +Configurable Error Injection
> +============================
> +
> +Overview
> +--------
> +
> +Configurable error injection allows injecting specific block layer status codes
> +for ranges of a block device.  Error can be injected unconditional, or with a

                                  Errors can be injected unconditionally or with a

> +given probability.
> +
> +To use configurable error injection, CONFIG_FAIL_MAKE_REQUEST must be enabled.
> +
> +The only interface is the error_injection debugfs file, which is created for
> +each registered gendisk.  Writes to this file are used to create or delete rules
> +and reads return a list of the current error injection sites.

[snip]

-- 
~Randy

[PATCH 9/9] block: move the fail request code

[Christoph Hellwig]

Keep all error injection in one place, and out of line for the main
I/O submission fast path.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c        | 37 ++-----------------------------------
 block/error-injection.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 04a392849ab0..7465dd291272 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -29,7 +29,6 @@
 #include <linux/swap.h>
 #include <linux/writeback.h>
 #include <linux/task_io_accounting_ops.h>
-#include <linux/fault-inject.h>
 #include <linux/list_sort.h>
 #include <linux/delay.h>
 #include <linux/ratelimit.h>
@@ -534,32 +533,6 @@ bool blk_get_queue(struct request_queue *q)
 }
 EXPORT_SYMBOL(blk_get_queue);
 
-#ifdef CONFIG_FAIL_MAKE_REQUEST
-
-static DECLARE_FAULT_ATTR(fail_make_request);
-
-static int __init setup_fail_make_request(char *str)
-{
-	return setup_fault_attr(&fail_make_request, str);
-}
-__setup("fail_make_request=", setup_fail_make_request);
-
-bool should_fail_request(unsigned int bytes)
-{
-	return should_fail(&fail_make_request, bytes);
-}
-
-static int __init fail_make_request_debugfs(void)
-{
-	struct dentry *dir = fault_create_debugfs_attr("fail_make_request",
-						NULL, &fail_make_request);
-
-	return PTR_ERR_OR_ZERO(dir);
-}
-
-late_initcall(fail_make_request_debugfs);
-#endif /* CONFIG_FAIL_MAKE_REQUEST */
-
 static inline void bio_check_ro(struct bio *bio)
 {
 	if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) {
@@ -764,14 +737,8 @@ static void __submit_bio_noacct_mq(struct bio *bio)
 
 void submit_bio_noacct_nocheck(struct bio *bio, bool split)
 {
-	if (unlikely(may_fail_bio(bio))) {
-		if (blk_error_inject(bio))
-			return;
-		if (should_fail_request(bio->bi_iter.bi_size)) {
-			bio_io_error(bio);
-			return;
-		}
-	}
+	if (unlikely(may_fail_bio(bio)) && blk_error_inject(bio))
+		return;
 
 	blk_cgroup_bio_start(bio);
 
diff --git a/block/error-injection.c b/block/error-injection.c
index dc0420c4eb58..45f2454d0bca 100644
--- a/block/error-injection.c
+++ b/block/error-injection.c
@@ -4,6 +4,7 @@
  */
 #include <linux/debugfs.h>
 #include <linux/blkdev.h>
+#include <linux/fault-inject.h>
 #include <linux/parser.h>
 #include <linux/seq_file.h>
 #include "blk.h"
@@ -47,6 +48,13 @@ bool __blk_error_inject(struct bio *bio)
 		}
 	}
 	rcu_read_unlock();
+
+	/* legacy I/O error injection */
+	if (should_fail_request(bio->bi_iter.bi_size)) {
+		bio_io_error(bio);
+		return true;
+	}
+
 	return false;
 }
 
@@ -297,3 +305,25 @@ void blk_error_injection_exit(struct gendisk *disk)
 {
 	error_inject_removall(disk);
 }
+
+static DECLARE_FAULT_ATTR(fail_make_request);
+
+bool should_fail_request(unsigned int bytes)
+{
+	return should_fail(&fail_make_request, bytes);
+}
+
+static int __init setup_fail_make_request(char *str)
+{
+	return setup_fault_attr(&fail_make_request, str);
+}
+__setup("fail_make_request=", setup_fail_make_request);
+
+static int __init fail_make_request_debugfs(void)
+{
+	struct dentry *dir = fault_create_debugfs_attr("fail_make_request",
+						NULL, &fail_make_request);
+
+	return PTR_ERR_OR_ZERO(dir);
+}
+late_initcall(fail_make_request_debugfs);
-- 
2.53.0

Re: configurable block error injection

[Keith Busch]

On Tue, Jun 02, 2026 at 07:45:32AM +0200, Christoph Hellwig wrote:
> Hi all,
> 
> this series adds a new configurable block error injection facility.
> We already have a few to inject block errors, but unfortunately most
> of them are either not very useful or hard to use, or both:

Looks great! I just have some comments on patch 8/9, but for the rest:

Reviewed-by: Keith Busch <kbusch@kernel.org>

Re: configurable block error injection

[Daniel Gomez]

On 02/06/2026 07.45, Christoph Hellwig wrote:
> Hi all,
> 
> this series adds a new configurable block error injection facility.
> We already have a few to inject block errors, but unfortunately most
> of them are either not very useful or hard to use, or both:
> 
>  - The fail_make_request failure injection point can't distinguish
>    different commands, different ranges in the file and can only injection
>    plain I/O errors.
>  - the should_fail_bio 'dynamic' failure injection has all the same issues
>    as fail_make_request
>  - dm-error can only fail all command in the table using BLK_STS_IOERR
>    and requires setting up a new block device
>  - dm-flakey and dm-dust allow all kinds of configurability, but still
>    don't have good error selection, no good support for non-read/write
>    commands and are limited to the dm table alignment requirements,
>    which for zoned devices enforces setting them up for an entire zone.
>    They also once again require setting up a stacked block device,
>    which is really annoying in harnesses like xfstests
> 
> This series adds a new debugfs-based block layer error injection
> that allows to configure what operations and ranges the injection
> applied to, and what status to return.  It also allows to configure a
> failure ratio similar to the xfs errortag injection.

I wonder if the block layer would be interested in moving block error
injection off the should_fail() fault injection framework and extending
the ALLOW_ERROR_INJECTION annotation instead and offloading all the
debugfs configuration logic (block/error-injection.c) into eBPF?

I talked about moderr [1] at LPC 2025. It's a simple error injection
tool in eBPF for the module subsystem. The suggested direction there was
to generalize the tool to ideally to no tool at all, and leverage
bpftrace to describe the error injection conditions a given
subsystem needs to be tested under. That would let blktests, for
example, absorb that and simplify the configuration logic this series
adds in the kernel for debugfs.

A previous attempt to add inline error injection [2] was rejected as too
intrusive / source-polluting; the eBPF approach solves that, since the
injection logic lives in a standalone tool/script rather than in the
kernel sources.

What do you guys think?

[1] https://lpc.events/event/19/contributions/2204/
[2] https://lore.kernel.org/all/20210512064629.13899-1-mcgrof@kernel.org/

Re: configurable block error injection

[Christoph Hellwig]

On Tue, Jun 02, 2026 at 11:58:25AM +0200, Daniel Gomez wrote:
> I wonder if the block layer would be interested in moving block error
> injection off the should_fail() fault injection framework and extending
> the ALLOW_ERROR_INJECTION annotation instead and offloading all the
> debugfs configuration logic (block/error-injection.c) into eBPF?

I've looked into plain ALLOW_ERROR_INJECTION-based injection and it
is not very useful.  I didn't even now eBPF could use it, but I
looked into other eBPF injections and at least for my uses cases
it was a bit of a mess.  I'd have to allow access to certain bio
fields and would have create a stable UAPI for commands and status
using the fake BTF struct access which really would not be a good
idea here as we need to be able to change internals.  Additionally
having fully BTF-enabled toolchains in test VMs is not great either.

I've also not actually found any good map type for range lookups,
which is kinda essential here.

> I talked about moderr [1] at LPC 2025. It's a simple error injection
> tool in eBPF for the module subsystem. The suggested direction there was
> to generalize the tool to ideally to no tool at all, and leverage
> bpftrace to describe the error injection conditions a given
> subsystem needs to be tested under. That would let blktests, for
> example, absorb that and simplify the configuration logic this series
> adds in the kernel for debugfs.

I don't think pulling in ebpftrace for simple error injection is a
winning proposition..

> 
> A previous attempt to add inline error injection [2] was rejected as too
> intrusive / source-polluting;

I'm not sure a single hand waivy comment counts as rejection, although
I'm not a huge fan of setup_fault_attr - it makes a mess of debugfs and
creates a lot of boilerplate for a single not very much configurable call
site.  That might be ok for something like the make_request case
(although I think we can do better as shown in this patch), but for
making random functions fail it is a lot of overhead.  These injections
points also are not anywhere near stable enough to be exposed.