[v2] btrfs: adjust error jump position

Commit Message

Hao Peng Jan. 7, 2023, 10 a.m. UTC

From: Peng Hao <[e-mail]>

Since 'em' has been set to NULL, you can jump directly to out_err.

Signed-off-by: Peng Hao <[e-mail]>
---
 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

                                            logical, &geom);
--
2.27.0

Comments

Qu Wenruo Jan. 7, 2023, 10:23 a.m. UTC | #1

On 2023/1/7 18:00, Hao Peng wrote:
> From: Peng Hao <[e-mail]>
> 
> Since 'em' has been set to NULL, you can jump directly to out_err.

Then why not remove out_err tag completely? Since free_extent_map() can 
handle empty pointers, and only keep one exit tag is helpful.
Otherwise it provides no benefit and even compiler can optimize it better.

Thus I'd recommend consider the benefit and put it into the commit 
message (especially under your company's name).

Thanks,
Qu
> 
> Signed-off-by: Peng Hao <[e-mail]>
> ---
>   fs/btrfs/inode.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 0e516aefbf51..0ee2e82d6be2 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7997,7 +7997,7 @@ static void btrfs_submit_direct(const struct
> iomap_iter *iter,
>                  if (IS_ERR(em)) {
>                          status = errno_to_blk_status(PTR_ERR(em));
>                          em = NULL;
> -                       goto out_err_em;
> +                       goto out_err;
>                  }
>                  ret = btrfs_get_io_geometry(fs_info, em, btrfs_op(dio_bio),
>                                              logical, &geom);
> --
> 2.27.0

David Sterba Jan. 9, 2023, 8:05 p.m. UTC | #2

On Sat, Jan 07, 2023 at 06:23:46PM +0800, Qu Wenruo wrote:
> 
> 
> On 2023/1/7 18:00, Hao Peng wrote:
> > From: Peng Hao <[e-mail]>
> > 
> > Since 'em' has been set to NULL, you can jump directly to out_err.
> 
> Then why not remove out_err tag completely? Since free_extent_map() can 
> handle empty pointers, and only keep one exit tag is helpful.
> Otherwise it provides no benefit and even compiler can optimize it better.

You're right that free_extent_map can handle NULL but I'd rather keep
the pattern consistent so the cleanup labels and the functions that
cause the error are properly nested.

David Sterba Jan. 9, 2023, 8:14 p.m. UTC | #3

On Sat, Jan 07, 2023 at 06:00:19PM +0800, Hao Peng wrote:
> From: Peng Hao <[e-mail]>
> 
> Since 'em' has been set to NULL, you can jump directly to out_err.
> 
> Signed-off-by: Peng Hao <[e-mail]>

With updated changelog added to misc-next, thanks.

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 0e516aefbf51..0ee2e82d6be2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7997,7 +7997,7 @@  static void btrfs_submit_direct(const struct
iomap_iter *iter,
                if (IS_ERR(em)) {
                        status = errno_to_blk_status(PTR_ERR(em));
                        em = NULL;
-                       goto out_err_em;
+                       goto out_err;
                }
                ret = btrfs_get_io_geometry(fs_info, em, btrfs_op(dio_bio),