diff options
author | Josef Bacik <jbacik@fb.com> | 2018-07-20 14:46:10 -0400 |
---|---|---|
committer | David Sterba <dsterba@suse.com> | 2018-08-06 07:12:58 -0400 |
commit | 3c4276936f6fbe52884b4ea4e6cc120b890a0f9f (patch) | |
tree | 5efe9f1d1fc3ce8ac71b742ef466790996110473 | |
parent | 97aff912a2fa75555641303d42bd4c723a98df5d (diff) |
Btrfs: fix btrfs_write_inode vs delayed iput deadlock
We recently ran into the following deadlock involving
btrfs_write_inode():
[ +0.005066] __schedule+0x38e/0x8c0
[ +0.007144] schedule+0x36/0x80
[ +0.006447] bit_wait+0x11/0x60
[ +0.006446] __wait_on_bit+0xbe/0x110
[ +0.007487] ? bit_wait_io+0x60/0x60
[ +0.007319] __inode_wait_for_writeback+0x96/0xc0
[ +0.009568] ? autoremove_wake_function+0x40/0x40
[ +0.009565] inode_wait_for_writeback+0x21/0x30
[ +0.009224] evict+0xb0/0x190
[ +0.006099] iput+0x1a8/0x210
[ +0.006103] btrfs_run_delayed_iputs+0x73/0xc0
[ +0.009047] btrfs_commit_transaction+0x799/0x8c0
[ +0.009567] btrfs_write_inode+0x81/0xb0
[ +0.008008] __writeback_single_inode+0x267/0x320
[ +0.009569] writeback_sb_inodes+0x25b/0x4e0
[ +0.008702] wb_writeback+0x102/0x2d0
[ +0.007487] wb_workfn+0xa4/0x310
[ +0.006794] ? wb_workfn+0xa4/0x310
[ +0.007143] process_one_work+0x150/0x410
[ +0.008179] worker_thread+0x6d/0x520
[ +0.007490] kthread+0x12c/0x160
[ +0.006620] ? put_pwq_unlocked+0x80/0x80
[ +0.008185] ? kthread_park+0xa0/0xa0
[ +0.007484] ? do_syscall_64+0x53/0x150
[ +0.007837] ret_from_fork+0x29/0x40
Writeback calls:
btrfs_write_inode
btrfs_commit_transaction
btrfs_run_delayed_iputs
If iput() is called on that same inode, evict() will wait for writeback
forever.
btrfs_write_inode() was originally added way back in 4730a4bc5bf3
("btrfs_dirty_inode") to support O_SYNC writes. However, ->write_inode()
hasn't been used for O_SYNC since 148f948ba877 ("vfs: Introduce new
helpers for syncing after writing to O_SYNC file or IS_SYNC inode"), so
btrfs_write_inode() is actually unnecessary (and leads to a bunch of
unnecessary commits). Get rid of it, which also gets rid of the
deadlock.
CC: stable@vger.kernel.org # 3.2+
Signed-off-by: Josef Bacik <jbacik@fb.com>
[Omar: new commit message]
Signed-off-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: David Sterba <dsterba@suse.com>
-rw-r--r-- | fs/btrfs/inode.c | 26 | ||||
-rw-r--r-- | fs/btrfs/super.c | 1 |
2 files changed, 0 insertions, 27 deletions
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 4955e04da4c8..472457795486 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c | |||
@@ -6021,32 +6021,6 @@ err: | |||
6021 | return ret; | 6021 | return ret; |
6022 | } | 6022 | } |
6023 | 6023 | ||
6024 | int btrfs_write_inode(struct inode *inode, struct writeback_control *wbc) | ||
6025 | { | ||
6026 | struct btrfs_root *root = BTRFS_I(inode)->root; | ||
6027 | struct btrfs_trans_handle *trans; | ||
6028 | int ret = 0; | ||
6029 | bool nolock = false; | ||
6030 | |||
6031 | if (test_bit(BTRFS_INODE_DUMMY, &BTRFS_I(inode)->runtime_flags)) | ||
6032 | return 0; | ||
6033 | |||
6034 | if (btrfs_fs_closing(root->fs_info) && | ||
6035 | btrfs_is_free_space_inode(BTRFS_I(inode))) | ||
6036 | nolock = true; | ||
6037 | |||
6038 | if (wbc->sync_mode == WB_SYNC_ALL) { | ||
6039 | if (nolock) | ||
6040 | trans = btrfs_join_transaction_nolock(root); | ||
6041 | else | ||
6042 | trans = btrfs_join_transaction(root); | ||
6043 | if (IS_ERR(trans)) | ||
6044 | return PTR_ERR(trans); | ||
6045 | ret = btrfs_commit_transaction(trans); | ||
6046 | } | ||
6047 | return ret; | ||
6048 | } | ||
6049 | |||
6050 | /* | 6024 | /* |
6051 | * This is somewhat expensive, updating the tree every time the | 6025 | * This is somewhat expensive, updating the tree every time the |
6052 | * inode changes. But, it is most likely to find the inode in cache. | 6026 | * inode changes. But, it is most likely to find the inode in cache. |
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index efe8b03ce380..67de3c0fc85b 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c | |||
@@ -2344,7 +2344,6 @@ static const struct super_operations btrfs_super_ops = { | |||
2344 | .sync_fs = btrfs_sync_fs, | 2344 | .sync_fs = btrfs_sync_fs, |
2345 | .show_options = btrfs_show_options, | 2345 | .show_options = btrfs_show_options, |
2346 | .show_devname = btrfs_show_devname, | 2346 | .show_devname = btrfs_show_devname, |
2347 | .write_inode = btrfs_write_inode, | ||
2348 | .alloc_inode = btrfs_alloc_inode, | 2347 | .alloc_inode = btrfs_alloc_inode, |
2349 | .destroy_inode = btrfs_destroy_inode, | 2348 | .destroy_inode = btrfs_destroy_inode, |
2350 | .statfs = btrfs_statfs, | 2349 | .statfs = btrfs_statfs, |