On Thu, Apr 05, 2018 at 11:45:55AM -0700, Liu Bo wrote:
> On Thu, Apr 5, 2018 at 9:11 AM, David Sterba <dste...@suse.cz> wrote:
> > On Sat, Mar 31, 2018 at 06:11:56AM +0800, Liu Bo wrote:
> >> Currently if some fatal errors occur, like all IO get -EIO, resources
> >> would be cleaned up when
> >> a) transaction is being committed or
> >> b) BTRFS_FS_STATE_ERROR is set
> >>
> >> However, in some rare cases, resources may be left alone after transaction
> >> gets aborted and umount may run into some ASSERT(), e.g.
> >> ASSERT(list_empty(&block_group->dirty_list));
> >>
> >> For case a), in btrfs_commit_transaciton(), there're several places at the
> >> beginning where we just call btrfs_end_transaction() without cleaning up
> >> resources.  For case b), it is possible that the trans handle doesn't have
> >> any dirty stuff, then only trans hanlde is marked as aborted while
> >> BTRFS_FS_STATE_ERROR is not set, so resources remain in memory.
> >>
> >> This makes btrfs also check BTRFS_FS_STATE_TRANS_ABORTED to make sure that
> >> all resources won't stay in memory after umount.
> >>
> >> Signed-off-by: Liu Bo <bo....@linux.alibaba.com>
> >
> > Is it possible that the following stactrace could be caused by the
> > missing check? It roughly matches what you describe (ie. close_ctree and
> > unreleased resources). This is from generic/475, that does some error
> > injection:
> >
> > [16991.455178] WARNING: CPU: 6 PID: 23518 at fs/btrfs/extent-tree.c:9896 
> > btrfs_free_block_groups+0x2c8/0x420 [btrfs]
> >
> 
> Hmm...I don't think so, while running 475, the one I got pretty stable is
> ASSERT(list_empty(&block_group->dirty_list));

There's a number of things that 475 catches so this might depend on
timing, memory, disks etc.

> And I did see this warning a few times, but I thought that was due to
> the new flag (ZERO) of fallocate for which we had fixes from Filipe,
> not sure if they've been merged?

Merged to 4.15:

* f27451f22996687 Btrfs: add support for fallocate's zero range operation
* 9f13ce743b1bd4e Btrfs: fix missing inode i_size update after zero range 
operation
--
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

Reply via email to