| Message ID | 20250408122933.121056-3-frank.li@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] btrfs: use BTRFS_PATH_AUTO_FREE in insert_balance_item() | expand |
On Tue, Apr 8, 2025 at 1:22 PM Yangtao Li <frank.li@vivo.com> wrote: > > Handle insert_empty_item errors by adding explicit > btrfs_abort_transaction/btrfs_end_transaction calls. But why should we do it? More below. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > fs/btrfs/volumes.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 375d92720e17..347c475028e0 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -3733,7 +3733,7 @@ static int insert_balance_item(struct btrfs_fs_info *fs_info, > BTRFS_PATH_AUTO_FREE(path); > struct extent_buffer *leaf; > struct btrfs_key key; > - int ret, err; > + int ret; > > path = btrfs_alloc_path(); > if (!path) > @@ -3749,8 +3749,11 @@ static int insert_balance_item(struct btrfs_fs_info *fs_info, > > ret = btrfs_insert_empty_item(trans, root, path, &key, > sizeof(*item)); > - if (ret) > - goto out; > + if (ret) { > + btrfs_abort_transaction(trans, ret); > + btrfs_end_transaction(trans); A transaction abort will turn the fs into RO mode, and it's meant to be used when we can't proceed with changes to the fs after we did partial changes, to avoid leaving things in an inconsistent state. Here we don't abort because we haven't done any changes before using the transaction handle, so an abort is pointless and will turn the fs into RO mode unnecessarily. Thanks. > + return ret; > + } > > leaf = path->nodes[0]; > item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_balance_item); > @@ -3764,11 +3767,8 @@ static int insert_balance_item(struct btrfs_fs_info *fs_info, > btrfs_cpu_balance_args_to_disk(&disk_bargs, &bctl->sys); > btrfs_set_balance_sys(leaf, item, &disk_bargs); > btrfs_set_balance_flags(leaf, item, bctl->flags); > -out: > - err = btrfs_commit_transaction(trans); > - if (err && !ret) > - ret = err; > - return ret; > + > + return btrfs_commit_transaction(trans); > } > > static int del_balance_item(struct btrfs_fs_info *fs_info) > -- > 2.39.0 > >
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 375d92720e17..347c475028e0 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -3733,7 +3733,7 @@ static int insert_balance_item(struct btrfs_fs_info *fs_info, BTRFS_PATH_AUTO_FREE(path); struct extent_buffer *leaf; struct btrfs_key key; - int ret, err; + int ret; path = btrfs_alloc_path(); if (!path) @@ -3749,8 +3749,11 @@ static int insert_balance_item(struct btrfs_fs_info *fs_info, ret = btrfs_insert_empty_item(trans, root, path, &key, sizeof(*item)); - if (ret) - goto out; + if (ret) { + btrfs_abort_transaction(trans, ret); + btrfs_end_transaction(trans); + return ret; + } leaf = path->nodes[0]; item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_balance_item); @@ -3764,11 +3767,8 @@ static int insert_balance_item(struct btrfs_fs_info *fs_info, btrfs_cpu_balance_args_to_disk(&disk_bargs, &bctl->sys); btrfs_set_balance_sys(leaf, item, &disk_bargs); btrfs_set_balance_flags(leaf, item, bctl->flags); -out: - err = btrfs_commit_transaction(trans); - if (err && !ret) - ret = err; - return ret; + + return btrfs_commit_transaction(trans); } static int del_balance_item(struct btrfs_fs_info *fs_info)
Handle insert_empty_item errors by adding explicit btrfs_abort_transaction/btrfs_end_transaction calls. Signed-off-by: Yangtao Li <frank.li@vivo.com> --- fs/btrfs/volumes.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)