On Thu, Jul 21, 2016 at 02:33:19PM -0700, Liu Bo wrote:

quoted

quoted

update_block_group() is the only producer to add block group cache to
dirty_bgs list, and if btrfs_run_delayed_refs() aborts, the transaction
is aborted, so seems that there won't be anyone manipulating dirty_bgs
list, am I missing?

No, the dirty_bgs processing is safe I think.  My concern is with the cache
inode which we iput()

I think iput() is OK, we're doing iput() on block group cache on the io_bgs
list, where all block groups's inodes has been igrab()'d.  If others are
messing around with our cache inode, they should have their own igrab,
too.

quoted

quoted

Another point is that when we fail on btrfs_start_dirty_block_groups(),
btrfs_commit_transaction() won't get to cleanup_transaction error
handling,

Right, because we don't actually finish the commit.  Someone will eventually
though ;)

Hmm yes, it's possible that there's a concurrent commit transaction
running.  If that's not true, we may still resort to
btrfs_error_commit_super(), other than that, I don't see who could
commit/cleanup the transaction after entering into BTRFS_FS_STATE_ERROR
state.

What's the resume of this patch? I don't see a followup patch or a (to
me) clear yes/no whether to merge it. Please let me know, thanks.