| Message ID | 20200716151719.3967-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Remove done label in writepage_delalloc | expand |
On 16/07/2020 17:17, Nikolay Borisov wrote: > Since there is not common cleanup run after the label it makes it somewhat > redundant. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/extent_io.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index a76b7da91aa6..e6d1d46ae384 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -3445,8 +3445,7 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode, > * started, so we don't want to return > 0 unless > * things are going well. > */ > - ret = ret < 0 ? ret : -EIO; > - goto done; > + return ret < 0 ? ret : -EIO; > } > /* > * delalloc_end is already one less than the total length, so > @@ -3478,10 +3477,7 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode, > return 1; > } > > - ret = 0; > - > -done: > - return ret; > + return 0; > } I thought David doesn't like direct returns in loops? /me thinks this is easier to understand though Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Fri, Jul 17, 2020 at 07:31:22AM +0000, Johannes Thumshirn wrote: > On 16/07/2020 17:17, Nikolay Borisov wrote: > > Since there is not common cleanup run after the label it makes it somewhat > > redundant. > > > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > > --- > > fs/btrfs/extent_io.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > > index a76b7da91aa6..e6d1d46ae384 100644 > > --- a/fs/btrfs/extent_io.c > > +++ b/fs/btrfs/extent_io.c > > @@ -3445,8 +3445,7 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode, > > * started, so we don't want to return > 0 unless > > * things are going well. > > */ > > - ret = ret < 0 ? ret : -EIO; > > - goto done; > > + return ret < 0 ? ret : -EIO; > > } > > /* > > * delalloc_end is already one less than the total length, so > > @@ -3478,10 +3477,7 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode, > > return 1; > > } > > > > - ret = 0; > > - > > -done: > > - return ret; > > + return 0; > > } > > I thought David doesn't like direct returns in loops? > > /me thinks this is easier to understand though I don't but try to evaluate each patch if it makes sense and the code is readable and does not diverge too much from patterns we have elsewhere. This one looks ok, so I'll add it to misc-next.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index a76b7da91aa6..e6d1d46ae384 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3445,8 +3445,7 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode, * started, so we don't want to return > 0 unless * things are going well. */ - ret = ret < 0 ? ret : -EIO; - goto done; + return ret < 0 ? ret : -EIO; } /* * delalloc_end is already one less than the total length, so @@ -3478,10 +3477,7 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode, return 1; } - ret = 0; - -done: - return ret; + return 0; } /*
Since there is not common cleanup run after the label it makes it somewhat redundant. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/extent_io.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) -- 2.17.1