Re: [PATCH 06/10] ext4: switch to using blk_next_discard_bio directly

Keith Busch
2024-03-07T16:13:11Z

On Thu, Mar 07, 2024 at 08:11:53AM -0700, Christoph Hellwig wrote:
> @@ -3840,12 +3840,16 @@ static inline int ext4_issue_discard(struct super_block *sb,
> trace_ext4_discard_blocks(sb,
> (unsigned long long) discard_block, count);
> if (biop) {

Does this 'if' case even need to exist? It looks unreachable since there
are only two callers of ext4_issue_discard(), and they both set 'biop'
to NULL. It looks like the last remaining caller using 'biop' was
removed with 55cdd0af2bc5ffc ("ext4: get discard out of jbd2 commit
kthread contex")

> - return __blkdev_issue_discard(sb->s_bdev,
> - (sector_t)discard_block << (sb->s_blocksize_bits - 9),
> - (sector_t)count << (sb->s_blocksize_bits - 9),
> - GFP_NOFS, biop);
> - } else
> - return sb_issue_discard(sb, discard_block, count, GFP_NOFS, 0);
> + unsigned int sshift = (sb->s_blocksize_bits - SECTOR_SHIFT);
> + sector_t sector = (sector_t)discard_block << sshift;
> + sector_t nr_sects = (sector_t)count << sshift;
> +
> + while (blk_next_discard_bio(sb->s_bdev, biop, &sector,
> + &nr_sects, GFP_NOFS))
> + ;

This pattern is repeated often in this series, so perhaps a helper
function for this common use case.