On Thu, Oct 20, 2016 at 09:04:23AM +0800, Qu Wenruo wrote:
> >> + /* Clear all free space cache inodes and its extent data */
> >> + while (1) {
> >> + bg_cache = btrfs_lookup_first_block_group(fs_info, current);
> >> + if (!bg_cache)
> >> + break;
> >> + ret = btrfs_clear_free_space_cache(fs_info, bg_cache);
> >
> > The function can fail for a lot of reasons, what would be the filesystem
> > state when we exit here? Some of the inodes could be cleared completely,
> > the last one partially. The function copes with a missing inode item
> > but I don't know how many other intermediate states could be left.
>
> If we exit here, no damage for the filesystem will be done, since we are
> protected by transaction.
>
> As you can find, in btrfs_clear_free_space_cache(),
> it will only commit transaction if we fully cleaned the whole inode and
> its free space header.
Ah I see, each blockgroup is killed inside a transaction. Then there's
one more to invalidate the super block cache generation.
> So we're quite safe here, free space header and inode are cleaned
> atomically.
>
> PS: We really need btrfs_abort_transaction(), or when we exit abnormally
> we will get a lot of backtrace/warning on uncommitted transaction.
Yeah, the userspace is lacking behind kernel code in the transaction
error handling. Patches welcome, this might be a big change all over the
place, so I suggest to do it in smaller batches.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
