On Tue, Apr 09, 2019 at 04:38:41AM -0700, Matthew Wilcox wrote:
> > +++ b/fs/btrfs/inode.c
> > @@ -7919,7 +7918,7 @@ static void btrfs_retry_endio(struct bio *bio)
> > struct bio_vec *bvec;
> > int uptodate;
> > int ret;
> > - int i;
> > + int i = 0;
> > struct bvec_iter_all iter_all;
> >
> > if (bio->bi_status)
> > @@ -7934,7 +7933,7 @@ static void btrfs_retry_endio(struct bio *bio)
> > failure_tree = &BTRFS_I(inode)->io_failure_tree;
> >
> > ASSERT(!bio_flagged(bio, BIO_CLONED));
> > - bio_for_each_segment_all(bvec, bio, i, iter_all) {
> > + bio_for_each_segment_all(bvec, bio, iter_all) {
> > ret = __readpage_endio_check(inode, io_bio, i, bvec->bv_page,
> > bvec->bv_offset, done->start,
> > bvec->bv_len);
> > @@ -7946,6 +7945,7 @@ static void btrfs_retry_endio(struct bio *bio)
> > bvec->bv_offset);
> > else
> > uptodate = 0;
> > + i++;
> > }
>
> I'd be tempted to instead:
>
> @@ -7935,7 +7935,7 @@ static void btrfs_retry_endio(struct bio *bio)
>
> ASSERT(!bio_flagged(bio, BIO_CLONED));
> bio_for_each_segment_all(bvec, bio, i, iter_all) {
> - ret = __readpage_endio_check(inode, io_bio, i, bvec->bv_page,
> + ret = __readpage_endio_check(inode, io_bio, i++, bvec->bv_page,
> bvec->bv_offset, done->start,
> bvec->bv_len);
> if (!ret)
>
> (i is used nowhere else in this loop, and it's a mercifully short loop with
> no breaks or continues).
I'd rather keep the separate statement than having parameters with
sideefects.
