Skip to content

Commit bf7ba8e

Browse files
josefbacikkdave
authored andcommitted
btrfs: fix deadlock with fsync+fiemap+transaction commit
We are hitting the following deadlock in production occasionally Task 1 Task 2 Task 3 Task 4 Task 5 fsync(A) start trans start commit falloc(A) lock 5m-10m start trans wait for commit fiemap(A) lock 0-10m wait for 5m-10m (have 0-5m locked) have btrfs_need_log_full_commit !full_sync wait_ordered_extents finish_ordered_io(A) lock 0-5m DEADLOCK We have an existing dependency of file extent lock -> transaction. However in fsync if we tried to do the fast logging, but then had to fall back to committing the transaction, we will be forced to call btrfs_wait_ordered_range() to make sure all of our extents are updated. This creates a dependency of transaction -> file extent lock, because btrfs_finish_ordered_io() will need to take the file extent lock in order to run the ordered extents. Fix this by stopping the transaction if we have to do the full commit and we attempted to do the fast logging. Then attach to the transaction and commit it if we need to. CC: stable@vger.kernel.org # 5.15+ Reviewed-by: Filipe Manana <fdmanana@suse.com> Signed-off-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent 97e8663 commit bf7ba8e

File tree

1 file changed

+52
-15
lines changed

1 file changed

+52
-15
lines changed

fs/btrfs/file.c

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2322,25 +2322,62 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
23222322
*/
23232323
btrfs_inode_unlock(inode, BTRFS_ILOCK_MMAP);
23242324

2325-
if (ret != BTRFS_NO_LOG_SYNC) {
2325+
if (ret == BTRFS_NO_LOG_SYNC) {
2326+
ret = btrfs_end_transaction(trans);
2327+
goto out;
2328+
}
2329+
2330+
/* We successfully logged the inode, attempt to sync the log. */
2331+
if (!ret) {
2332+
ret = btrfs_sync_log(trans, root, &ctx);
23262333
if (!ret) {
2327-
ret = btrfs_sync_log(trans, root, &ctx);
2328-
if (!ret) {
2329-
ret = btrfs_end_transaction(trans);
2330-
goto out;
2331-
}
2332-
}
2333-
if (!full_sync) {
2334-
ret = btrfs_wait_ordered_range(inode, start, len);
2335-
if (ret) {
2336-
btrfs_end_transaction(trans);
2337-
goto out;
2338-
}
2334+
ret = btrfs_end_transaction(trans);
2335+
goto out;
23392336
}
2340-
ret = btrfs_commit_transaction(trans);
2341-
} else {
2337+
}
2338+
2339+
/*
2340+
* At this point we need to commit the transaction because we had
2341+
* btrfs_need_log_full_commit() or some other error.
2342+
*
2343+
* If we didn't do a full sync we have to stop the trans handle, wait on
2344+
* the ordered extents, start it again and commit the transaction. If
2345+
* we attempt to wait on the ordered extents here we could deadlock with
2346+
* something like fallocate() that is holding the extent lock trying to
2347+
* start a transaction while some other thread is trying to commit the
2348+
* transaction while we (fsync) are currently holding the transaction
2349+
* open.
2350+
*/
2351+
if (!full_sync) {
23422352
ret = btrfs_end_transaction(trans);
2353+
if (ret)
2354+
goto out;
2355+
ret = btrfs_wait_ordered_range(inode, start, len);
2356+
if (ret)
2357+
goto out;
2358+
2359+
/*
2360+
* This is safe to use here because we're only interested in
2361+
* making sure the transaction that had the ordered extents is
2362+
* committed. We aren't waiting on anything past this point,
2363+
* we're purely getting the transaction and committing it.
2364+
*/
2365+
trans = btrfs_attach_transaction_barrier(root);
2366+
if (IS_ERR(trans)) {
2367+
ret = PTR_ERR(trans);
2368+
2369+
/*
2370+
* We committed the transaction and there's no currently
2371+
* running transaction, this means everything we care
2372+
* about made it to disk and we are done.
2373+
*/
2374+
if (ret == -ENOENT)
2375+
ret = 0;
2376+
goto out;
2377+
}
23432378
}
2379+
2380+
ret = btrfs_commit_transaction(trans);
23442381
out:
23452382
ASSERT(list_empty(&ctx.list));
23462383
err = file_check_and_advance_wb_err(file);

0 commit comments

Comments
 (0)