[PATCH v2 0/6] Make blkdev_issue_discard() submit aligned discard requests

[PATCH v2 0/6] Make blkdev_issue_discard() submit aligned discard requests

[Bart Van Assche]

Users of blkdev_issue_discard(), e.g. blk_ioctl_discard(), expect that 
this function generates discard request(s) that satisfy block driver 
alignment requirements. Hence modify blk_bio_discard_split() such that 
the submitted discard requests are aligned.

Changes compared to v1:
- Moved the code to split discard requests from blkdev_issue_discard()
   into blk_bio_discard_split().

The patches in this series are:
0001-block-Introduce-blk_round_sect_down.patch
0002-block-Introduce-bio_add_zero_pages.patch
0003-block-Optimize-__blkdev_issue_zeroout.patch
0004-block-Make-blkdev_issue_discard-submit-aligned-disca.patch
0005-brd-Split-bio-if-needed.patch
0006-brd-Accept-non-aligned-discard-requests-with-size-ze.patch

[PATCH 1/6] block: Introduce blk_round_sect_down()

[Bart Van Assche]

Move the code for rounding down a sector into a new function.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Dmitry Monakhov <dmonakhov@openvz.org>
---
 block/blk-lib.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 9ebf653..2f4c6b2 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -26,6 +26,26 @@ static void bio_batch_end_io(struct bio *bio)
 	bio_put(bio);
 }
 
+/*
+ * Return the largest number that is less than or equal to @s and for which
+ * the remainder of the division by @granularity is @alignment.
+ */
+static sector_t blk_round_sect_down(sector_t s, u32 granularity, u32 alignment)
+{
+	sector_t tmp = s, res = s;
+	u32 remainder;
+
+	WARN_ON_ONCE(alignment >= granularity);
+
+	remainder = sector_div(tmp, granularity);
+	if (remainder == alignment)
+		return res;
+	res -= remainder - alignment;
+	if (remainder < alignment)
+		res -= granularity;
+	return min(res, s);
+}
+
 /**
  * blkdev_issue_discard - queue a discard
  * @bdev:	blockdev to issue discard for
@@ -73,7 +93,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 	blk_start_plug(&plug);
 	while (nr_sects) {
 		unsigned int req_sects;
-		sector_t end_sect, tmp;
+		sector_t end_sect;
 
 		bio = bio_alloc(gfp_mask, 1);
 		if (!bio) {
@@ -89,12 +109,10 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 		 * misaligned, stop the discard at the previous aligned sector.
 		 */
 		end_sect = sector + req_sects;
-		tmp = end_sect;
-		if (req_sects < nr_sects &&
-		    sector_div(tmp, granularity) != alignment) {
-			end_sect = end_sect - alignment;
-			sector_div(end_sect, granularity);
-			end_sect = end_sect * granularity + alignment;
+		if (req_sects < nr_sects) {
+			end_sect = blk_round_sect_down(end_sect, granularity,
+						       alignment);
+			WARN_ON_ONCE(end_sect < sector);
 			req_sects = end_sect - sector;
 		}
 
-- 
2.1.4

[PATCH 2/6] block: Introduce bio_add_zero_pages()

[Bart Van Assche]

Move the code for adding one or more zero pages into a new function.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Dmitry Monakhov <dmonakhov@openvz.org>
---
 block/blk-lib.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 2f4c6b2..f44ec95 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -219,6 +219,22 @@ int blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
 }
 EXPORT_SYMBOL(blkdev_issue_write_same);
 
+static void bio_add_zero_pages(struct bio *bio, sector_t nr_sects)
+{
+	unsigned sz;
+	int ret;
+
+	/* Avoid that bi_size overflows */
+	nr_sects = min_t(sector_t, nr_sects, UINT_MAX >> 9);
+
+	for (; nr_sects != 0; nr_sects -= ret >> 9) {
+		sz = min_t(unsigned, PAGE_SIZE >> 9, nr_sects) << 9;
+		ret = bio_add_page(bio, ZERO_PAGE(0), sz, 0);
+		if (ret < sz)
+			break;
+	}
+}
+
 /**
  * blkdev_issue_zeroout - generate number of zero filed write bios
  * @bdev:	blockdev to issue
@@ -236,7 +252,6 @@ static int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 	int ret;
 	struct bio *bio;
 	struct bio_batch bb;
-	unsigned int sz;
 	DECLARE_COMPLETION_ONSTACK(wait);
 
 	atomic_set(&bb.done, 1);
@@ -256,16 +271,9 @@ static int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 		bio->bi_bdev   = bdev;
 		bio->bi_end_io = bio_batch_end_io;
 		bio->bi_private = &bb;
-
-		while (nr_sects != 0) {
-			sz = min((sector_t) PAGE_SIZE >> 9 , nr_sects);
-			ret = bio_add_page(bio, ZERO_PAGE(0), sz << 9, 0);
-			nr_sects -= ret >> 9;
-			sector += ret >> 9;
-			if (ret < (sz << 9))
-				break;
-		}
-		ret = 0;
+		bio_add_zero_pages(bio, nr_sects);
+		nr_sects -= bio->bi_iter.bi_size >> 9;
+		sector += bio->bi_iter.bi_size >> 9;
 		atomic_inc(&bb.done);
 		submit_bio(WRITE, bio);
 	}
-- 
2.1.4

[PATCH 3/6] block: Optimize __blkdev_issue_zeroout()

[Bart Van Assche]

Avoid to confuse SCSI target software by ensuring that the WRITE SAME
data buffer length matches the block size.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Dmitry Monakhov <dmonakhov@openvz.org>
---
 block/blk-lib.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index f44ec95..1a60b3f 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -249,7 +249,9 @@ static void bio_add_zero_pages(struct bio *bio, sector_t nr_sects)
 static int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 				  sector_t nr_sects, gfp_t gfp_mask)
 {
-	int ret;
+	int ret = 0;
+	struct request_queue *q = bdev_get_queue(bdev);
+	unsigned req_sects;
 	struct bio *bio;
 	struct bio_batch bb;
 	DECLARE_COMPLETION_ONSTACK(wait);
@@ -258,7 +260,6 @@ static int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 	bb.error = 0;
 	bb.wait = &wait;
 
-	ret = 0;
 	while (nr_sects != 0) {
 		bio = bio_alloc(gfp_mask,
 				min(nr_sects, (sector_t)BIO_MAX_PAGES));
@@ -271,9 +272,11 @@ static int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
 		bio->bi_bdev   = bdev;
 		bio->bi_end_io = bio_batch_end_io;
 		bio->bi_private = &bb;
-		bio_add_zero_pages(bio, nr_sects);
-		nr_sects -= bio->bi_iter.bi_size >> 9;
-		sector += bio->bi_iter.bi_size >> 9;
+		bio_add_zero_pages(bio, q->limits.logical_block_size);
+		req_sects = min_t(sector_t, nr_sects, UINT_MAX >> 9);
+		bio->bi_iter.bi_size = req_sects << 9;
+		nr_sects -= req_sects;
+		sector += req_sects;
 		atomic_inc(&bb.done);
 		submit_bio(WRITE, bio);
 	}
-- 
2.1.4

[PATCH 4/6] block: Make blkdev_issue_discard() submit aligned discard requests

[Bart Van Assche]

Split discard requests as follows:
* If the start sector is not aligned, an initial write request up
  to the first aligned sector.
* A discard request from the first aligned sector in the range up
  to the last aligned sector in the discarded range.
* If the end sector is not aligned, a final write request from the
  last aligned sector up to the end.

Note: if the start and/or end sectors are not aligned and if the
range is small enough the discard request will be submitted with
bi_size == 0.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Dmitry Monakhov <dmonakhov@openvz.org>
---
 block/blk-lib.c   |  4 ++--
 block/blk-merge.c | 55 ++++++++++++++++++++++++++++++-------------------------
 block/blk.h       |  3 +++
 3 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 1a60b3f..da0a744 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -30,7 +30,7 @@ static void bio_batch_end_io(struct bio *bio)
  * Return the largest number that is less than or equal to @s and for which
  * the remainder of the division by @granularity is @alignment.
  */
-static sector_t blk_round_sect_down(sector_t s, u32 granularity, u32 alignment)
+sector_t blk_round_sect_down(sector_t s, u32 granularity, u32 alignment)
 {
 	sector_t tmp = s, res = s;
 	u32 remainder;
@@ -219,7 +219,7 @@ int blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
 }
 EXPORT_SYMBOL(blkdev_issue_write_same);
 
-static void bio_add_zero_pages(struct bio *bio, sector_t nr_sects)
+void bio_add_zero_pages(struct bio *bio, sector_t nr_sects)
 {
 	unsigned sz;
 	int ret;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index e01405a..2ce5a78 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -14,42 +14,47 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
 					 struct bio_set *bs,
 					 unsigned *nsegs)
 {
+	struct bio *wr;
 	unsigned int max_discard_sectors, granularity;
 	int alignment;
-	sector_t tmp;
-	unsigned split_sectors;
+	sector_t start, start_r, end, end_r, skip;
 
 	*nsegs = 1;
 
 	/* Zero-sector (unknown) and one-sector granularities are the same.  */
 	granularity = max(q->limits.discard_granularity >> 9, 1U);
-
+	alignment = (q->limits.discard_alignment >> 9) % granularity;
 	max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
-	max_discard_sectors -= max_discard_sectors % granularity;
-
-	if (unlikely(!max_discard_sectors)) {
-		/* XXX: warn */
-		return NULL;
-	}
-
-	if (bio_sectors(bio) <= max_discard_sectors)
-		return NULL;
-
-	split_sectors = max_discard_sectors;
+	WARN_ON_ONCE(max_discard_sectors == 0);
 
 	/*
-	 * If the next starting sector would be misaligned, stop the discard at
-	 * the previous aligned sector.
+	 * If the start or end sector are misaligned, issue a write same
+	 * same request if the discard_zeroes_data flag has been set.
 	 */
-	alignment = (q->limits.discard_alignment >> 9) % granularity;
-
-	tmp = bio->bi_iter.bi_sector + split_sectors - alignment;
-	tmp = sector_div(tmp, granularity);
-
-	if (split_sectors > tmp)
-		split_sectors -= tmp;
-
-	return bio_split(bio, split_sectors, GFP_NOIO, bs);
+	start = bio->bi_iter.bi_sector;
+	start_r = blk_round_sect_down(start, granularity, alignment);
+	end = start + min(max_discard_sectors, bio_sectors(bio));
+	end_r = blk_round_sect_down(end, granularity, alignment);
+	if (start == start_r && start < end_r) {
+		if (end == end_r && bio_sectors(bio) == end_r - start)
+			return NULL;
+		return bio_split(bio, end_r - start, GFP_NOIO, bs);
+	} else if (q->limits.discard_zeroes_data) {
+		end = min(end, start_r + granularity);
+		wr = bio_alloc_bioset(GFP_NOIO, end - start, bs);
+		if (WARN_ON_ONCE(!wr))
+			return NULL;
+		wr->bi_rw = REQ_WRITE;
+		wr->bi_iter.bi_sector = start;
+		wr->bi_bdev = bio->bi_bdev;
+		bio_add_zero_pages(wr, end - start);
+		bio_advance(bio, wr->bi_iter.bi_size);
+		return wr;
+	} else {
+		skip = (min(start_r + granularity, end) - start) << 9;
+		bio_advance(bio, skip);
+		return NULL;
+	}
 }
 
 static struct bio *blk_bio_write_same_split(struct request_queue *q,
diff --git a/block/blk.h b/block/blk.h
index c43926d..e043c34 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -36,6 +36,9 @@ extern struct kmem_cache *request_cachep;
 extern struct kobj_type blk_queue_ktype;
 extern struct ida blk_queue_ida;
 
+sector_t blk_round_sect_down(sector_t s, u32 granularity, u32 alignment);
+void bio_add_zero_pages(struct bio *bio, sector_t nr_sects);
+
 static inline struct blk_flush_queue *blk_get_flush_queue(
 		struct request_queue *q, struct blk_mq_ctx *ctx)
 {
-- 
2.1.4

[PATCH 5/6] brd: Split bio if needed

[Bart Van Assche]

If a bio is submitted to the brd driver that does not satisfy the
requirements of this driver, split the bio.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Dmitry Monakhov <dmonakhov@openvz.org>
---
 block/blk-merge.c   | 2 +-
 drivers/block/brd.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 2ce5a78..c158577 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -39,7 +39,7 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
 		if (end == end_r && bio_sectors(bio) == end_r - start)
 			return NULL;
 		return bio_split(bio, end_r - start, GFP_NOIO, bs);
-	} else if (q->limits.discard_zeroes_data) {
+	} else if (q->limits.discard_zeroes_data && start < end) {
 		end = min(end, start_r + granularity);
 		wr = bio_alloc_bioset(GFP_NOIO, end - start, bs);
 		if (WARN_ON_ONCE(!wr))
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 1914c63..7e0a54e 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -336,6 +336,8 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
 	if (bio_end_sector(bio) > get_capacity(bdev->bd_disk))
 		goto io_error;
 
+	blk_queue_split(q, &bio, q->bio_split);
+
 	if (unlikely(bio->bi_rw & REQ_DISCARD)) {
 		if (sector & ((PAGE_SIZE >> SECTOR_SHIFT) - 1) ||
 		    bio->bi_iter.bi_size & ~PAGE_MASK)
-- 
2.1.4

[PATCH 6/6] brd: Accept non-aligned discard requests with size zero

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Dmitry Monakhov <dmonakhov@openvz.org>
---
 drivers/block/brd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 7e0a54e..a757c7b 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -339,8 +339,9 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
 	blk_queue_split(q, &bio, q->bio_split);
 
 	if (unlikely(bio->bi_rw & REQ_DISCARD)) {
-		if (sector & ((PAGE_SIZE >> SECTOR_SHIFT) - 1) ||
-		    bio->bi_iter.bi_size & ~PAGE_MASK)
+		if (bio->bi_iter.bi_size &&
+		    (sector & ((PAGE_SIZE >> SECTOR_SHIFT) - 1) ||
+		     bio->bi_iter.bi_size & ~PAGE_MASK))
 			goto io_error;
 		discard_from_brd(brd, sector, bio->bi_iter.bi_size);
 		goto out;
-- 
2.1.4

Re: [PATCH 3/6] block: Optimize __blkdev_issue_zeroout()

[Bart Van Assche]

On 12/17/2015 12:41 PM, Bart Van Assche wrote:
> Avoid to confuse SCSI target software by ensuring that the WRITE SAME
> data buffer length matches the block size.

(replying to my own e-mail)

Please ignore this patch - it is wrong. Apparently my jet lag has not 
yet completely disappeared ...

Bart.

Re: [PATCH 1/6] block: Introduce blk_round_sect_down()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/6] block: Introduce bio_add_zero_pages()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>