On Mon, Jan 28, 2019 at 06:57:41PM +0200, Nikolay Borisov wrote:
+static void btrfs_cleanup_pending_block_groups(struct btrfs_trans_handle *trans) +{
struct btrfs_fs_info *fs_info = trans->fs_info;struct btrfs_block_group_cache *block_group;while (!list_empty(&trans->new_bgs)) {block_group = list_first_entry(&trans->new_bgs,struct btrfs_block_group_cache,bg_list);btrfs_delayed_refs_rsv_release(fs_info, 1);list_del_init(&block_group->bg_list);}+}
This is much cleaner and understandable, thanks.
nit:Can't we use list_for_each_entry_safe though and save the explicit list_first_entry. IMO this is fine here since the transaction is aborted hence no new pending bgs can be added to the ->new_bgs list. In any case:
@@ -1898,12 +1898,9 @@ static inline int btrfs_start_delalloc_flush(struct btrfs_fs_info *fs_info) static void btrfs_cleanup_pending_block_groups(struct btrfs_trans_handle *trans) { struct btrfs_fs_info *fs_info = trans->fs_info; - struct btrfs_block_group_cache *block_group; + struct btrfs_block_group_cache *block_group, *tmp;
- while (!list_empty(&trans->new_bgs)) { - block_group = list_first_entry(&trans->new_bgs, - struct btrfs_block_group_cache, - bg_list); + list_for_each_entry_safe(block_group, tmp, &trans->new_bgs, bg_list) { btrfs_delayed_refs_rsv_release(fs_info, 1); list_del_init(&block_group->bg_list);
Looks better than the version I copied from create_pending_bgs, the transaction is going to be freed soon so there should be no new entries, indeed.