Re: [PATCH] block: don't use for-inside-for in bio_for_each_segment_all

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux