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

[Christoph Hellwig <hch@lst.de>]

On Thu, Mar 07, 2024 at 09:13:01AM -0700, Keith Busch wrote:
> 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")

Yeah.  I didn't really want to dig so far into code I don't know well
for this series, though :)

> > +		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.

Well, it's 2-3 lines vs 1 line for a much better interface.