On Wed, Oct 31, 2018 at 10:06:08AM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osan...@fb.com>
> 
> There's a race between close_ctree() and cleaner_kthread().
> close_ctree() sets btrfs_fs_closing(), and the cleaner stops when it
> sees it set, but this is racy; the cleaner might have already checked
> the bit and could be cleaning stuff. In particular, if it deletes unused
> block groups, it will create delayed iputs for the free space cache
> inodes. As of "btrfs: don't run delayed_iputs in commit", we're no
> longer running delayed iputs after a commit. Therefore, if the cleaner
> creates more delayed iputs after delayed iputs are run in
> btrfs_commit_super(), we will leak inodes on unmount and get a busy
> inode crash from the VFS.
> 
> Fix it by parking the cleaner

Ouch, that's IMO wrong way to fix it. The bug is on a higher level,
we're missing a commit or clean up data structures. Messing with state
of a thread would be the last thing I'd try after proving that it's not
possible to fix in the logic of btrfs itself.

The shutdown sequence in close_tree is quite tricky and we've had bugs
there. The interdependencies of thread and data structures and other
subsystems cannot have loops that could not find an ordering that will
not leak something.

It's not a big problem if some step is done more than once, like
committing or cleaning up some other structures if we know that
it could create new.

Reply via email to