[PATCH] ext4: issue aligned discards

[PATCH] ext4: issue aligned discards

[Harshad Shirwadkar]

Ext4 before this patch can issue discards without respecting block
device's discard alignment. Such a discard results in EIO and
kernel logs.

Verified that there were no regressions in xfstests smoke tests.

Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 fs/ext4/mballoc.c      | 32 +++++++++++++++++++++-----------
 include/linux/blkdev.h |  7 +++++++
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 30d5d97548c4..a591a3ab93d3 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2786,20 +2786,30 @@ static inline int ext4_issue_discard(struct super_block *sb,
 		ext4_group_t block_group, ext4_grpblk_t cluster, int count,
 		struct bio **biop)
 {
-	ext4_fsblk_t discard_block;
-
-	discard_block = (EXT4_C2B(EXT4_SB(sb), cluster) +
-			 ext4_group_first_block_no(sb, block_group));
-	count = EXT4_C2B(EXT4_SB(sb), count);
+	unsigned long long discard_start, discard_len, alignment, granularity,
+			aligned_discard_start;
+
+	granularity = max(bdev_discard_granularity(sb->s_bdev), 1 << 9);
+	alignment = max_t(int, bdev_logical_block_size(sb->s_bdev),
+			  bdev_discard_alignment(sb->s_bdev));
+	discard_start = (EXT4_C2B(EXT4_SB(sb), cluster) +
+			 ext4_group_first_block_no(sb, block_group)) <<
+			sb->s_blocksize_bits;
+	discard_len = EXT4_C2B(EXT4_SB(sb), count) << sb->s_blocksize_bits;
+	aligned_discard_start = round_up(discard_start, alignment);
+	discard_len -= min(discard_len, aligned_discard_start - discard_start);
+	discard_len = round_down(discard_len, granularity);
+	if (discard_len >> 9 == 0)
+		return 0;
 	trace_ext4_discard_blocks(sb,
-			(unsigned long long) discard_block, count);
-	if (biop) {
+				  aligned_discard_start >> sb->s_blocksize_bits,
+				  discard_len >> (sb->s_blocksize_bits));
+	if (biop)
 		return __blkdev_issue_discard(sb->s_bdev,
-			(sector_t)discard_block << (sb->s_blocksize_bits - 9),
-			(sector_t)count << (sb->s_blocksize_bits - 9),
+			aligned_discard_start >> 9, discard_len >> 9,
 			GFP_NOFS, 0, biop);
-	} else
-		return sb_issue_discard(sb, discard_block, count, GFP_NOFS, 0);
+	return sb_issue_discard(sb, aligned_discard_start, discard_len,
+				GFP_NOFS, 0);
 }
 
 static void ext4_free_data_in_buddy(struct super_block *sb,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8fd900998b4e..f448b3498336 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1431,6 +1431,13 @@ static inline int queue_limit_discard_alignment(struct queue_limits *lim, sector
 	return offset << SECTOR_SHIFT;
 }
 
+static inline int bdev_discard_granularity(struct block_device *bdev)
+{
+	struct request_queue *q = bdev_get_queue(bdev);
+
+	return q ? q->limits.discard_granularity : 0;
+}
+
 static inline int bdev_discard_alignment(struct block_device *bdev)
 {
 	struct request_queue *q = bdev_get_queue(bdev);
-- 
2.27.0.278.ge193c7cf3a9-goog

Re: [PATCH] ext4: issue aligned discards

[Christoph Hellwig]

On Fri, Jun 05, 2020 at 03:28:19PM -0700, Harshad Shirwadkar wrote:
> Ext4 before this patch can issue discards without respecting block
> device's discard alignment. Such a discard results in EIO and
> kernel logs.

No, that is not how discard works.  The granularity is a hint and
blk_bio_discard_split already does all the work to align to it.  If
you have a make_request based driver that doesn't do that you need
to fix that driver instead.

Re: [PATCH] ext4: issue aligned discards

[harshad shirwadkar]

On Mon, Jun 8, 2020 at 12:35 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Jun 05, 2020 at 03:28:19PM -0700, Harshad Shirwadkar wrote:
> > Ext4 before this patch can issue discards without respecting block
> > device's discard alignment. Such a discard results in EIO and
> > kernel logs.
>
> No, that is not how discard works.  The granularity is a hint and
> blk_bio_discard_split already does all the work to align to it.  If
> you have a make_request based driver that doesn't do that you need
> to fix that driver instead.

Thanks for the clarification, my commit message was wrong. Fixing the
driver instead is the right solution here. But, if doing discard
alignment in Ext4 can increase the chances of discards being
successful, is there any harm in doing it in Ext4 too?