| Message ID | 20240926115935.20631-1-jth@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: remove code duplication in ordered extent finishing | expand |
On Thu, Sep 26, 2024 at 1:27 PM Johannes Thumshirn <jth@kernel.org> wrote: > > From: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > Remove the duplicated transaction joining, block reserve setting and raid > extent inserting in btrfs_finish_ordered_extent(). > > While at it, also abort the transaction in case inserting a RAID > stripe-tree entry fails. > > Suggested-by: Naohiro Aota <naohiro.aota@wdc.com> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/inode.c | 45 ++++++++++++++++----------------------------- > 1 file changed, 16 insertions(+), 29 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 353fb58c83da..35a03a671fc6 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -3068,34 +3068,6 @@ int btrfs_finish_one_ordered(struct btrfs_ordered_extent *ordered_extent) > goto out; > } > > - if (test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags)) { > - BUG_ON(!list_empty(&ordered_extent->list)); /* Logic error */ > - > - btrfs_inode_safe_disk_i_size_write(inode, 0); > - if (freespace_inode) > - trans = btrfs_join_transaction_spacecache(root); > - else > - trans = btrfs_join_transaction(root); > - if (IS_ERR(trans)) { > - ret = PTR_ERR(trans); > - trans = NULL; > - goto out; > - } > - trans->block_rsv = &inode->block_rsv; > - ret = btrfs_update_inode_fallback(trans, inode); > - if (ret) /* -ENOMEM or corruption */ > - btrfs_abort_transaction(trans, ret); > - > - ret = btrfs_insert_raid_extent(trans, ordered_extent); > - if (ret) > - btrfs_abort_transaction(trans, ret); > - > - goto out; > - } > - > - clear_bits |= EXTENT_LOCKED; > - lock_extent(io_tree, start, end, &cached_state); > - > if (freespace_inode) > trans = btrfs_join_transaction_spacecache(root); > else > @@ -3109,8 +3081,23 @@ int btrfs_finish_one_ordered(struct btrfs_ordered_extent *ordered_extent) > trans->block_rsv = &inode->block_rsv; > > ret = btrfs_insert_raid_extent(trans, ordered_extent); > - if (ret) > + if (ret) { > + btrfs_abort_transaction(trans, ret); > + goto out; > + } > + > + if (test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags)) { > + BUG_ON(!list_empty(&ordered_extent->list)); /* Logic error */ > + > + btrfs_inode_safe_disk_i_size_write(inode, 0); > + ret = btrfs_update_inode_fallback(trans, inode); > + if (ret) /* -ENOMEM or corruption */ While at it we can change the comment to comply with the preferred style, to leave it on the line above or inside the if: if (ret) { /* -ENOMEM or corruption */ btrfs_abort_transaction(trans, ret); } Same for the other comment after the BUG_ON() (and we could change the BUG_ON() to ASSERT() plus abort transaction maybe, but that probably as a separate patch). Otherwise it looks fine, thanks. Reviewed-by: Filipe Manana <fdmanana@suse.com> > + btrfs_abort_transaction(trans, ret); > goto out; > + } > + > + clear_bits |= EXTENT_LOCKED; > + lock_extent(io_tree, start, end, &cached_state); > > if (test_bit(BTRFS_ORDERED_COMPRESSED, &ordered_extent->flags)) > compress_type = ordered_extent->compress_type; > -- > 2.43.0 > >
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 353fb58c83da..35a03a671fc6 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -3068,34 +3068,6 @@ int btrfs_finish_one_ordered(struct btrfs_ordered_extent *ordered_extent) goto out; } - if (test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags)) { - BUG_ON(!list_empty(&ordered_extent->list)); /* Logic error */ - - btrfs_inode_safe_disk_i_size_write(inode, 0); - if (freespace_inode) - trans = btrfs_join_transaction_spacecache(root); - else - trans = btrfs_join_transaction(root); - if (IS_ERR(trans)) { - ret = PTR_ERR(trans); - trans = NULL; - goto out; - } - trans->block_rsv = &inode->block_rsv; - ret = btrfs_update_inode_fallback(trans, inode); - if (ret) /* -ENOMEM or corruption */ - btrfs_abort_transaction(trans, ret); - - ret = btrfs_insert_raid_extent(trans, ordered_extent); - if (ret) - btrfs_abort_transaction(trans, ret); - - goto out; - } - - clear_bits |= EXTENT_LOCKED; - lock_extent(io_tree, start, end, &cached_state); - if (freespace_inode) trans = btrfs_join_transaction_spacecache(root); else @@ -3109,8 +3081,23 @@ int btrfs_finish_one_ordered(struct btrfs_ordered_extent *ordered_extent) trans->block_rsv = &inode->block_rsv; ret = btrfs_insert_raid_extent(trans, ordered_extent); - if (ret) + if (ret) { + btrfs_abort_transaction(trans, ret); + goto out; + } + + if (test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags)) { + BUG_ON(!list_empty(&ordered_extent->list)); /* Logic error */ + + btrfs_inode_safe_disk_i_size_write(inode, 0); + ret = btrfs_update_inode_fallback(trans, inode); + if (ret) /* -ENOMEM or corruption */ + btrfs_abort_transaction(trans, ret); goto out; + } + + clear_bits |= EXTENT_LOCKED; + lock_extent(io_tree, start, end, &cached_state); if (test_bit(BTRFS_ORDERED_COMPRESSED, &ordered_extent->flags)) compress_type = ordered_extent->compress_type;