make secure erase and write zeroes ioctls interruptible as well

make secure erase and write zeroes ioctls interruptible as well

[Christoph Hellwig]

Hi all,

Following discard in the last merge window, this series also makes secure
erase and discard interruptible by fatal signals.

The secure erase side is a straight port of the discard support.
Unfortunately I don't have a way to test it, so I'm adding the eMMC
maintainer as that is where the support originated so maybe they can
give it a spin?  (just do a blkdiscard -f -s /dev/<dev> and then Ctrl+C)

The write zeroes support is a bit different as it is more complex due to
the fallback code and there already is a helper taking flags that we
piggy back on.  This side has been extensively tested.

Diffstat:
 block/blk-lib.c        |  267 +++++++++++++++++++++++++++----------------------
 block/ioctl.c          |   44 +++++++-
 include/linux/bio.h    |    2 
 include/linux/blkdev.h |    1 
 4 files changed, 193 insertions(+), 121 deletions(-)

[PATCH 01/10] block: move secure erase checks into the ioctl handler

[Christoph Hellwig]

Most bio operations get basic sanity checking in submit_bio and anything
more complicated than that is done in the callers.  Secure erase is a bit
different from that in that a lot of checking is done in
blkdev_issue_secure_erase, and the specific errnos for that are returned
to userspace.  Move the checks that require specific errnos to the ioctl
handler instead, and just leave the basic sanity checking in submit_bio

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c | 7 -------
 block/ioctl.c   | 8 +++++++-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 442da9dad04213..4aabfc4a7eaa20 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -299,13 +299,6 @@ int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
 		max_sectors = UINT_MAX >> SECTOR_SHIFT;
 	max_sectors &= ~bs_mask;
 
-	if (max_sectors == 0)
-		return -EOPNOTSUPP;
-	if ((sector | nr_sects) & bs_mask)
-		return -EINVAL;
-	if (bdev_read_only(bdev))
-		return -EPERM;
-
 	blk_start_plug(&plug);
 	while (nr_sects) {
 		unsigned int len = min_t(sector_t, nr_sects, max_sectors);
diff --git a/block/ioctl.c b/block/ioctl.c
index d570e16958961e..f53121edb9a15f 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -163,6 +163,7 @@ static int blk_ioctl_discard(struct block_device *bdev, blk_mode_t mode,
 static int blk_ioctl_secure_erase(struct block_device *bdev, blk_mode_t mode,
 		void __user *argp)
 {
+	unsigned int bs_mask = bdev_logical_block_size(bdev) - 1;
 	uint64_t start, len;
 	uint64_t range[2];
 	int err;
@@ -171,12 +172,17 @@ static int blk_ioctl_secure_erase(struct block_device *bdev, blk_mode_t mode,
 		return -EBADF;
 	if (!bdev_max_secure_erase_sectors(bdev))
 		return -EOPNOTSUPP;
+	if (bdev_read_only(bdev))
+		return -EPERM;
 	if (copy_from_user(range, argp, sizeof(range)))
 		return -EFAULT;
 
 	start = range[0];
 	len = range[1];
-	if ((start & 511) || (len & 511))
+
+	if (!len)
+ 		return -EINVAL;
+	if ((start | len) & bs_mask)
 		return -EINVAL;
 	if (start + len > bdev_nr_bytes(bdev))
 		return -EINVAL;
-- 
2.43.0

[PATCH 02/10] block: factor out a bio_secure_erase_limit helper

[Christoph Hellwig]

Add a helper to size the maximum secure erase bio without exceeding the
queue limit or overlowing the bi_size member.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 4aabfc4a7eaa20..ff12c3d2de5aae 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -285,20 +285,22 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 }
 EXPORT_SYMBOL(blkdev_issue_zeroout);
 
+static sector_t bio_secure_erase_limit(struct block_device *bdev)
+{
+	sector_t bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
+
+	return min(bdev_max_secure_erase_sectors(bdev),
+		(UINT_MAX >> SECTOR_SHIFT) & ~bs_mask);
+}
+
 int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
 		sector_t nr_sects, gfp_t gfp)
 {
-	sector_t bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
-	unsigned int max_sectors = bdev_max_secure_erase_sectors(bdev);
+	unsigned int max_sectors = bio_secure_erase_limit(bdev);
 	struct bio *bio = NULL;
 	struct blk_plug plug;
 	int ret = 0;
 
-	/* make sure that "len << SECTOR_SHIFT" doesn't overflow */
-	if (max_sectors > UINT_MAX >> SECTOR_SHIFT)
-		max_sectors = UINT_MAX >> SECTOR_SHIFT;
-	max_sectors &= ~bs_mask;
-
 	blk_start_plug(&plug);
 	while (nr_sects) {
 		unsigned int len = min_t(sector_t, nr_sects, max_sectors);
-- 
2.43.0

[PATCH 03/10] block: add a blk_alloc_secure_erase_bio helper

[Christoph Hellwig]

Factor out a helper from blkdev_issue_secure_erase that chews off as much
as possible from a secure_erase range and allocates a bio for it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c     | 36 +++++++++++++++++++++++-------------
 include/linux/bio.h |  2 ++
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index ff12c3d2de5aae..cf4f0ee6b14503 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -293,26 +293,36 @@ static sector_t bio_secure_erase_limit(struct block_device *bdev)
 		(UINT_MAX >> SECTOR_SHIFT) & ~bs_mask);
 }
 
+struct bio *blk_alloc_secure_erase_bio(struct block_device *bdev,
+		sector_t *sector, sector_t *nr_sects, gfp_t gfp)
+{
+	sector_t bio_sects = min(*nr_sects, bio_secure_erase_limit(bdev));
+	struct bio *bio;
+
+	if (!bio_sects)
+		return NULL;
+	bio = bio_alloc(bdev, 0, REQ_OP_SECURE_ERASE, gfp);
+	if (!bio)
+		return NULL;
+	bio->bi_iter.bi_sector = *sector;
+	bio->bi_iter.bi_size = bio_sects << SECTOR_SHIFT;
+	*sector += bio_sects;
+	*nr_sects -= bio_sects;
+	cond_resched();
+	return bio;
+}
+
 int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
 		sector_t nr_sects, gfp_t gfp)
 {
-	unsigned int max_sectors = bio_secure_erase_limit(bdev);
-	struct bio *bio = NULL;
+	struct bio *bio = NULL, *next;
 	struct blk_plug plug;
 	int ret = 0;
 
 	blk_start_plug(&plug);
-	while (nr_sects) {
-		unsigned int len = min_t(sector_t, nr_sects, max_sectors);
-
-		bio = blk_next_bio(bio, bdev, 0, REQ_OP_SECURE_ERASE, gfp);
-		bio->bi_iter.bi_sector = sector;
-		bio->bi_iter.bi_size = len << SECTOR_SHIFT;
-
-		sector += len;
-		nr_sects -= len;
-		cond_resched();
-	}
+	while ((next = blk_alloc_secure_erase_bio(bdev, &sector, &nr_sects,
+			gfp)))
+		bio = bio_chain_and_submit(bio, next);
 	if (bio) {
 		ret = submit_bio_wait(bio);
 		bio_put(bio);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index d5379548d684e1..e1da5fe49baca1 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -835,5 +835,7 @@ struct bio *bio_chain_and_submit(struct bio *prev, struct bio *new);
 
 struct bio *blk_alloc_discard_bio(struct block_device *bdev,
 		sector_t *sector, sector_t *nr_sects, gfp_t gfp_mask);
+struct bio *blk_alloc_secure_erase_bio(struct block_device *bdev,
+		sector_t *sector, sector_t *nr_sects, gfp_t gfp);
 
 #endif /* __LINUX_BIO_H */
-- 
2.43.0

[PATCH 04/10] blk-lib: check for kill signal in ioctl BLKSECDISCARD

[Christoph Hellwig]

Secure Erase can access a significant capacity and take longer than the
user expected.  A user may change their mind about wanting to run that
command and attempt to kill the process and do something else with their
device. But since the task is uninterruptable, they have to wait for it
to finish, which could be many hours.

Open code blkdev_issue_secure_erase in the BLKSECDISCARD ioctl handler
and check for a fatal signal at each iteration so the user doesn't have
to wait for their regretted operation to complete naturally.

Heavily based on an earlier patch from Keith Busch.

Reported-by: Conrad Meyer <conradmeyer@meta.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/ioctl.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index f53121edb9a15f..45668a21cdb374 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -164,6 +164,9 @@ static int blk_ioctl_secure_erase(struct block_device *bdev, blk_mode_t mode,
 		void __user *argp)
 {
 	unsigned int bs_mask = bdev_logical_block_size(bdev) - 1;
+	struct bio *prev = NULL, *bio;
+	sector_t sector, nr_sects;
+	struct blk_plug plug;
 	uint64_t start, len;
 	uint64_t range[2];
 	int err;
@@ -187,11 +190,36 @@ static int blk_ioctl_secure_erase(struct block_device *bdev, blk_mode_t mode,
 	if (start + len > bdev_nr_bytes(bdev))
 		return -EINVAL;
 
+	sector = start >> SECTOR_SHIFT;
+	nr_sects = len >> SECTOR_SHIFT;
+
 	filemap_invalidate_lock(bdev->bd_mapping);
 	err = truncate_bdev_range(bdev, mode, start, start + len - 1);
-	if (!err)
-		err = blkdev_issue_secure_erase(bdev, start >> 9, len >> 9,
-						GFP_KERNEL);
+	if (err)
+		goto out_unlock;
+
+	blk_start_plug(&plug);
+	while (1) {
+		if (fatal_signal_pending(current)) {
+			if (prev)
+				bio_await_chain(prev);
+			err = -EINTR;
+			goto out_unplug;
+		}
+		bio = blk_alloc_secure_erase_bio(bdev, &sector, &nr_sects,
+				GFP_KERNEL);
+		if (!bio)
+			break;
+		prev = bio_chain_and_submit(prev, bio);
+	}
+	if (prev) {
+		err = submit_bio_wait(prev);
+		bio_put(prev);
+	}
+
+out_unplug:
+	blk_finish_plug(&plug);
+out_unlock:
 	filemap_invalidate_unlock(bdev->bd_mapping);
 	return err;
 }
-- 
2.43.0

[PATCH 05/10] block: factor out a blk_write_zeroes_limit helper

[Christoph Hellwig]

Contrary to the comment in __blkdev_issue_write_zeroes, nothing here
checks for a potential bi_size overflow.  Add a helper mirroring
the secure erase code for the check.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index cf4f0ee6b14503..f338709aef0c30 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -103,24 +103,28 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 }
 EXPORT_SYMBOL(blkdev_issue_discard);
 
+static sector_t bio_write_zeroes_limit(struct block_device *bdev)
+{
+	sector_t bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
+
+	return min(bdev_write_zeroes_sectors(bdev),
+		(UINT_MAX >> SECTOR_SHIFT) & ~bs_mask);
+}
+
 static int __blkdev_issue_write_zeroes(struct block_device *bdev,
 		sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
 		struct bio **biop, unsigned flags)
 {
 	struct bio *bio = *biop;
-	unsigned int max_sectors;
 
 	if (bdev_read_only(bdev))
 		return -EPERM;
-
-	/* Ensure that max_sectors doesn't overflow bi_size */
-	max_sectors = bdev_write_zeroes_sectors(bdev);
-
-	if (max_sectors == 0)
+	if (!bdev_write_zeroes_sectors(bdev))
 		return -EOPNOTSUPP;
 
 	while (nr_sects) {
-		unsigned int len = min_t(sector_t, nr_sects, max_sectors);
+		unsigned int len = min_t(sector_t, nr_sects,
+				bio_write_zeroes_limit(bdev));
 
 		bio = blk_next_bio(bio, bdev, 0, REQ_OP_WRITE_ZEROES, gfp_mask);
 		bio->bi_iter.bi_sector = sector;
-- 
2.43.0

[PATCH 06/10] block: remove the LBA alignment check in __blkdev_issue_zeroout

[Christoph Hellwig]

__blkdev_issue_zeroout is a purely kernel internal API and thus can rely
on the block layer sector alignment checks.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index f338709aef0c30..7aa7937c34201d 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -209,11 +209,6 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 		unsigned flags)
 {
 	int ret;
-	sector_t bs_mask;
-
-	bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
-	if ((sector | nr_sects) & bs_mask)
-		return -EINVAL;
 
 	ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp_mask,
 			biop, flags);
-- 
2.43.0

[PATCH 07/10] block: move read-only and supported checks into (__)blkdev_issue_zeroout

[Christoph Hellwig]

Move these checks out of the lower level helpers and into the higher level
ones to prepare for refactoring.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c | 51 ++++++++++++++++++++++---------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 7aa7937c34201d..10325e91560c40 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -111,17 +111,12 @@ static sector_t bio_write_zeroes_limit(struct block_device *bdev)
 		(UINT_MAX >> SECTOR_SHIFT) & ~bs_mask);
 }
 
-static int __blkdev_issue_write_zeroes(struct block_device *bdev,
+static void __blkdev_issue_write_zeroes(struct block_device *bdev,
 		sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
 		struct bio **biop, unsigned flags)
 {
 	struct bio *bio = *biop;
 
-	if (bdev_read_only(bdev))
-		return -EPERM;
-	if (!bdev_write_zeroes_sectors(bdev))
-		return -EOPNOTSUPP;
-
 	while (nr_sects) {
 		unsigned int len = min_t(sector_t, nr_sects,
 				bio_write_zeroes_limit(bdev));
@@ -138,7 +133,6 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev,
 	}
 
 	*biop = bio;
-	return 0;
 }
 
 /*
@@ -154,7 +148,7 @@ static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects)
 	return min(pages, (sector_t)BIO_MAX_VECS);
 }
 
-static int __blkdev_issue_zero_pages(struct block_device *bdev,
+static void __blkdev_issue_zero_pages(struct block_device *bdev,
 		sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
 		struct bio **biop)
 {
@@ -162,9 +156,6 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
 	int bi_size = 0;
 	unsigned int sz;
 
-	if (bdev_read_only(bdev))
-		return -EPERM;
-
 	while (nr_sects != 0) {
 		bio = blk_next_bio(bio, bdev, __blkdev_sectors_to_bio_pages(nr_sects),
 				   REQ_OP_WRITE, gfp_mask);
@@ -182,7 +173,6 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
 	}
 
 	*biop = bio;
-	return 0;
 }
 
 /**
@@ -208,15 +198,19 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 		sector_t nr_sects, gfp_t gfp_mask, struct bio **biop,
 		unsigned flags)
 {
-	int ret;
-
-	ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp_mask,
-			biop, flags);
-	if (ret != -EOPNOTSUPP || (flags & BLKDEV_ZERO_NOFALLBACK))
-		return ret;
+	if (bdev_read_only(bdev))
+		return -EPERM;
 
-	return __blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask,
-					 biop);
+	if (bdev_write_zeroes_sectors(bdev)) {
+		__blkdev_issue_write_zeroes(bdev, sector, nr_sects,
+				gfp_mask, biop, flags);
+	} else {
+		if (flags & BLKDEV_ZERO_NOFALLBACK)
+			return -EOPNOTSUPP;
+		__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask,
+				biop);
+	}
+	return 0;
 }
 EXPORT_SYMBOL(__blkdev_issue_zeroout);
 
@@ -245,21 +239,22 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 	bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
 	if ((sector | nr_sects) & bs_mask)
 		return -EINVAL;
+	if (bdev_read_only(bdev))
+		return -EPERM;
+	if ((flags & BLKDEV_ZERO_NOFALLBACK) && !try_write_zeroes)
+		return -EOPNOTSUPP;
 
 retry:
 	bio = NULL;
 	blk_start_plug(&plug);
 	if (try_write_zeroes) {
-		ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects,
-						  gfp_mask, &bio, flags);
-	} else if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
-		ret = __blkdev_issue_zero_pages(bdev, sector, nr_sects,
-						gfp_mask, &bio);
+		__blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp_mask,
+				&bio, flags);
 	} else {
-		/* No zeroing offload support */
-		ret = -EOPNOTSUPP;
+		__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask,
+				&bio);
 	}
-	if (ret == 0 && bio) {
+	if (bio) {
 		ret = submit_bio_wait(bio);
 		bio_put(bio);
 	}
-- 
2.43.0

[PATCH 08/10] block: refacto blkdev_issue_zeroout

[Christoph Hellwig]

Split out two well-defined helpers for hardware supported Write Zeroes
and manually writing zeroes using the Write command.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c | 94 +++++++++++++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 39 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 10325e91560c40..9585178a51a60c 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -135,6 +135,32 @@ static void __blkdev_issue_write_zeroes(struct block_device *bdev,
 	*biop = bio;
 }
 
+static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector,
+		sector_t nr_sects, gfp_t gfp, unsigned flags)
+{
+	struct bio *bio = NULL;
+	struct blk_plug plug;
+	int ret = 0;
+
+	blk_start_plug(&plug);
+	__blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp, &bio, flags);
+	if (bio) {
+		ret = submit_bio_wait(bio);
+		bio_put(bio);
+	}
+	blk_finish_plug(&plug);
+
+	/*
+	 * For some devices there is no non-destructive way to verify whether
+	 * WRITE ZEROES is actually supported.  These will clear the capability
+	 * on an I/O error, in which case we'll turn any error into
+	 * "not supported" here.
+	 */
+	if (ret && !bdev_write_zeroes_sectors(bdev))
+		return -EOPNOTSUPP;
+	return ret;
+}
+
 /*
  * Convert a number of 512B sectors to a number of pages.
  * The result is limited to a number of pages that can fit into a BIO.
@@ -175,6 +201,27 @@ static void __blkdev_issue_zero_pages(struct block_device *bdev,
 	*biop = bio;
 }
 
+static int blkdev_issue_zero_pages(struct block_device *bdev, sector_t sector,
+		sector_t nr_sects, gfp_t gfp, unsigned flags)
+{
+	struct bio *bio = NULL;
+	struct blk_plug plug;
+	int ret = 0;
+
+	if (flags & BLKDEV_ZERO_NOFALLBACK)
+		return -EOPNOTSUPP;
+
+	blk_start_plug(&plug);
+	__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp, &bio);
+	if (bio) {
+		ret = submit_bio_wait(bio);
+		bio_put(bio);
+	}
+	blk_finish_plug(&plug);
+
+	return ret;
+}
+
 /**
  * __blkdev_issue_zeroout - generate number of zero filed write bios
  * @bdev:	blockdev to issue
@@ -230,52 +277,21 @@ EXPORT_SYMBOL(__blkdev_issue_zeroout);
 int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 		sector_t nr_sects, gfp_t gfp_mask, unsigned flags)
 {
-	int ret = 0;
-	sector_t bs_mask;
-	struct bio *bio;
-	struct blk_plug plug;
-	bool try_write_zeroes = !!bdev_write_zeroes_sectors(bdev);
+	int ret;
 
-	bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
-	if ((sector | nr_sects) & bs_mask)
+	if ((sector | nr_sects) & ((bdev_logical_block_size(bdev) >> 9) - 1))
 		return -EINVAL;
 	if (bdev_read_only(bdev))
 		return -EPERM;
-	if ((flags & BLKDEV_ZERO_NOFALLBACK) && !try_write_zeroes)
-		return -EOPNOTSUPP;
 
-retry:
-	bio = NULL;
-	blk_start_plug(&plug);
-	if (try_write_zeroes) {
-		__blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp_mask,
-				&bio, flags);
-	} else {
-		__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask,
-				&bio);
-	}
-	if (bio) {
-		ret = submit_bio_wait(bio);
-		bio_put(bio);
-	}
-	blk_finish_plug(&plug);
-	if (ret && try_write_zeroes) {
-		if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
-			try_write_zeroes = false;
-			goto retry;
-		}
-		if (!bdev_write_zeroes_sectors(bdev)) {
-			/*
-			 * Zeroing offload support was indicated, but the
-			 * device reported ILLEGAL REQUEST (for some devices
-			 * there is no non-destructive way to verify whether
-			 * WRITE ZEROES is actually supported).
-			 */
-			ret = -EOPNOTSUPP;
-		}
+	if (bdev_write_zeroes_sectors(bdev)) {
+		ret = blkdev_issue_write_zeroes(bdev, sector, nr_sects,
+				gfp_mask, flags);
+		if (!ret)
+			return ret;
 	}
 
-	return ret;
+	return blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask, flags);
 }
 EXPORT_SYMBOL(blkdev_issue_zeroout);
 
-- 
2.43.0

[PATCH 09/10] block: limit the Write Zeroes to manually writing zeroes fallback

[Christoph Hellwig]

Only fall back from hardware Write Zeroes failures when
blkdev_issue_write_zeroes returns -EOPNOTSUPP;

Note that blkdev_issue_write_zeroes turns any failure into -EOPNOTSUPP
when the write zeroes queue limit has been cleared to 0, so this still
catches all I/O errors where the driver detected missing support
for the hardware acceleration.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 9585178a51a60c..95a22e7b1d3b85 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -287,7 +287,7 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 	if (bdev_write_zeroes_sectors(bdev)) {
 		ret = blkdev_issue_write_zeroes(bdev, sector, nr_sects,
 				gfp_mask, flags);
-		if (!ret)
+		if (ret != -EOPNOTSUPP)
 			return ret;
 	}
 
-- 
2.43.0

[PATCH 10/10] blk-lib: check for kill signal in ioctl BLKZEROOUT

[Christoph Hellwig]

Zeroout can access a significant capacity and take longer than the user
expected.  A user may change their mind about wanting to run that
command and attempt to kill the process and do something else with their
device. But since the task is uninterruptable, they have to wait for it
to finish, which could be many hours.

Add a new BLKDEV_ZERO_KILLABLE flag for blkdev_issue_zeroout that checks
for a fatal signal at each iteration so the user doesn't have to wait for
their regretted operation to complete naturally.

Heavily based on an earlier patch from Keith Busch.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-lib.c        | 66 +++++++++++++++++++++++++++---------------
 block/ioctl.c          |  2 +-
 include/linux/blkdev.h |  1 +
 3 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 95a22e7b1d3b85..729a2669681773 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -115,24 +115,27 @@ static void __blkdev_issue_write_zeroes(struct block_device *bdev,
 		sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
 		struct bio **biop, unsigned flags)
 {
-	struct bio *bio = *biop;
-
 	while (nr_sects) {
 		unsigned int len = min_t(sector_t, nr_sects,
 				bio_write_zeroes_limit(bdev));
+		struct bio *bio;
+
+		if ((flags & BLKDEV_ZERO_KILLABLE) &&
+		    fatal_signal_pending(current))
+			break;
 
-		bio = blk_next_bio(bio, bdev, 0, REQ_OP_WRITE_ZEROES, gfp_mask);
+		bio = bio_alloc(bdev, 0, REQ_OP_WRITE_ZEROES, gfp_mask);
 		bio->bi_iter.bi_sector = sector;
 		if (flags & BLKDEV_ZERO_NOUNMAP)
 			bio->bi_opf |= REQ_NOUNMAP;
 
 		bio->bi_iter.bi_size = len << SECTOR_SHIFT;
+		*biop = bio_chain_and_submit(*biop, bio);
+
 		nr_sects -= len;
 		sector += len;
 		cond_resched();
 	}
-
-	*biop = bio;
 }
 
 static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector,
@@ -145,6 +148,12 @@ static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector,
 	blk_start_plug(&plug);
 	__blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp, &bio, flags);
 	if (bio) {
+		if ((flags & BLKDEV_ZERO_KILLABLE) &&
+		    fatal_signal_pending(current)) {
+			bio_await_chain(bio);
+			blk_finish_plug(&plug);
+			return -EINTR;
+		}
 		ret = submit_bio_wait(bio);
 		bio_put(bio);
 	}
@@ -176,29 +185,34 @@ static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects)
 
 static void __blkdev_issue_zero_pages(struct block_device *bdev,
 		sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
-		struct bio **biop)
+		struct bio **biop, unsigned int flags)
 {
-	struct bio *bio = *biop;
-	int bi_size = 0;
-	unsigned int sz;
+	while (nr_sects) {
+		unsigned int nr_vecs = __blkdev_sectors_to_bio_pages(nr_sects);
+		struct bio *bio;
 
-	while (nr_sects != 0) {
-		bio = blk_next_bio(bio, bdev, __blkdev_sectors_to_bio_pages(nr_sects),
-				   REQ_OP_WRITE, gfp_mask);
+		bio = bio_alloc(bdev, nr_vecs, REQ_OP_WRITE, gfp_mask);
 		bio->bi_iter.bi_sector = sector;
 
-		while (nr_sects != 0) {
-			sz = min((sector_t) PAGE_SIZE, nr_sects << 9);
-			bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0);
-			nr_sects -= bi_size >> 9;
-			sector += bi_size >> 9;
-			if (bi_size < sz)
+		if ((flags & BLKDEV_ZERO_KILLABLE) &&
+		    fatal_signal_pending(current))
+			break;
+
+		do {
+			unsigned int len, added;
+
+			len = min_t(sector_t,
+				PAGE_SIZE, nr_sects << SECTOR_SHIFT);
+			added = bio_add_page(bio, ZERO_PAGE(0), len, 0);
+			if (added < len)
 				break;
-		}
+			nr_sects -= added >> SECTOR_SHIFT;
+			sector += added >> SECTOR_SHIFT;
+		} while (nr_sects);
+
+		*biop = bio_chain_and_submit(*biop, bio);
 		cond_resched();
 	}
-
-	*biop = bio;
 }
 
 static int blkdev_issue_zero_pages(struct block_device *bdev, sector_t sector,
@@ -212,8 +226,14 @@ static int blkdev_issue_zero_pages(struct block_device *bdev, sector_t sector,
 		return -EOPNOTSUPP;
 
 	blk_start_plug(&plug);
-	__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp, &bio);
+	__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp, &bio, flags);
 	if (bio) {
+		if ((flags & BLKDEV_ZERO_KILLABLE) &&
+		    fatal_signal_pending(current)) {
+			bio_await_chain(bio);
+			blk_finish_plug(&plug);
+			return -EINTR;
+		}
 		ret = submit_bio_wait(bio);
 		bio_put(bio);
 	}
@@ -255,7 +275,7 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 		if (flags & BLKDEV_ZERO_NOFALLBACK)
 			return -EOPNOTSUPP;
 		__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask,
-				biop);
+				biop, flags);
 	}
 	return 0;
 }
diff --git a/block/ioctl.c b/block/ioctl.c
index 45668a21cdb374..83899d06a9d30d 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -258,7 +258,7 @@ static int blk_ioctl_zeroout(struct block_device *bdev, blk_mode_t mode,
 		goto fail;
 
 	err = blkdev_issue_zeroout(bdev, start >> 9, len >> 9, GFP_KERNEL,
-				   BLKDEV_ZERO_NOUNMAP);
+				   BLKDEV_ZERO_NOUNMAP | BLKDEV_ZERO_KILLABLE);
 
 fail:
 	filemap_invalidate_unlock(bdev->bd_mapping);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4d0d4b83bc740f..538bb9b9b6cb32 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1100,6 +1100,7 @@ int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
 
 #define BLKDEV_ZERO_NOUNMAP	(1 << 0)  /* do not free blocks */
 #define BLKDEV_ZERO_NOFALLBACK	(1 << 1)  /* don't write explicit zeroes */
+#define BLKDEV_ZERO_KILLABLE	(1 << 2)  /* interruptible by fatal signals */
 
 extern int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 		sector_t nr_sects, gfp_t gfp_mask, struct bio **biop,
-- 
2.43.0

Re: make secure erase and write zeroes ioctls interruptible as well

[Martin K. Petersen]

Christoph,

> Following discard in the last merge window, this series also makes
> secure erase and discard interruptible by fatal signals.
>
> The secure erase side is a straight port of the discard support.
> Unfortunately I don't have a way to test it, so I'm adding the eMMC
> maintainer as that is where the support originated so maybe they can
> give it a spin? (just do a blkdiscard -f -s /dev/<dev> and then
> Ctrl+C)
>
> The write zeroes support is a bit different as it is more complex due
> to the fallback code and there already is a helper taking flags that
> we piggy back on. This side has been extensively tested.

Looks good to me.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: make secure erase and write zeroes ioctls interruptible as well

[Jens Axboe]

On 7/1/24 10:51 AM, Christoph Hellwig wrote:
> Hi all,
> 
> Following discard in the last merge window, this series also makes secure
> erase and discard interruptible by fatal signals.
> 
> The secure erase side is a straight port of the discard support.
> Unfortunately I don't have a way to test it, so I'm adding the eMMC
> maintainer as that is where the support originated so maybe they can
> give it a spin?  (just do a blkdiscard -f -s /dev/<dev> and then Ctrl+C)

In lieu of that, qemu does support mmc it looks like?

Wanted to get this queued up, but would probably be best to have
that tested first.

-- 
Jens Axboe

Re: make secure erase and write zeroes ioctls interruptible as well

[Christoph Hellwig]

On Fri, Jul 05, 2024 at 12:45:08AM -0600, Jens Axboe wrote:
> In lieu of that, qemu does support mmc it looks like?

Hmm, I can take a look.

> Wanted to get this queued up, but would probably be best to have
> that tested first.

The write zeroes patches do not depend on the secure erase patches, and
they are what people really care about.  Maybe just skip the secure
erase patches and apply the rest for now?

Re: (subset) make secure erase and write zeroes ioctls interruptible as well

[Jens Axboe]

On Mon, 01 Jul 2024 18:51:10 +0200, Christoph Hellwig wrote:
> Following discard in the last merge window, this series also makes secure
> erase and discard interruptible by fatal signals.
> 
> The secure erase side is a straight port of the discard support.
> Unfortunately I don't have a way to test it, so I'm adding the eMMC
> maintainer as that is where the support originated so maybe they can
> give it a spin?  (just do a blkdiscard -f -s /dev/<dev> and then Ctrl+C)
> 
> [...]

Applied, thanks!

[05/10] block: factor out a blk_write_zeroes_limit helper
        commit: 73a768d5f95533574bb8ace34eb683a88c40509e
[06/10] block: remove the LBA alignment check in __blkdev_issue_zeroout
        commit: ff760a8f0d09f4ba7574ae2ca8be987854f5246d
[07/10] block: move read-only and supported checks into (__)blkdev_issue_zeroout
        commit: f6eacb26541ad1eabc40d7e9f5cd86bae7dc0b46
[08/10] block: refacto blkdev_issue_zeroout
        commit: 99800ced26b9d87a918aa9824881bdb90a3c1b03
[09/10] block: limit the Write Zeroes to manually writing zeroes fallback
        commit: 39722a2f2bcd82bdecc226711412d88b54fcb05b
[10/10] blk-lib: check for kill signal in ioctl BLKZEROOUT
        commit: bf86bcdb40123ee99669ee91b67e023669433a1a

Best regards,
-- 
Jens Axboe

Re: make secure erase and write zeroes ioctls interruptible as well

[Jens Axboe]

On Fri, Jul 5, 2024 at 12:50?AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Jul 05, 2024 at 12:45:08AM -0600, Jens Axboe wrote:
> > In lieu of that, qemu does support mmc it looks like?
>
> Hmm, I can take a look.
>
> > Wanted to get this queued up, but would probably be best to have
> > that tested first.
>
> The write zeroes patches do not depend on the secure erase patches, and
> they are what people really care about.  Maybe just skip the secure
> erase patches and apply the rest for now?

Done - please just resend the secure erase bits when the mmc side is
happy.

-- 
Jens Axboe

Re: make secure erase and write zeroes ioctls interruptible as well

[Christoph Hellwig]

On Fri, Jul 05, 2024 at 12:45:08AM -0600, Jens Axboe wrote:
> In lieu of that, qemu does support mmc it looks like?

qemu does support mmc, but not the secure erase command.
virtio also has secure erase support, but that isn't implemented by
qemu either.