Re: [PATCH] btrfs: clean up pending block groups when transaction commit aborts

[David Sterba <dsterba@xxxxxxx>]

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.