All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: don't commit transaction for every subvol create
@ 2023-04-11 19:10 Sweet Tea Dorminy
  2023-04-12  1:45 ` Neal Gompa
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-11 19:10 UTC (permalink / raw)
  To: Chris Mason, Josef Bacik, David Sterba, linux-btrfs, kernel-team
  Cc: Sweet Tea Dorminy

Recently a Meta-internal workload encountered subvolume creation taking
up to 2s each, significantly slower than directory creation. As they
were hoping to be able to use subvolumes instead of directories, and
were looking to create hundreds, this was a significant issue. After
Josef investigated, it turned out to be due to the transaction commit
currently performed at the end of subvolume creation.

This change improves the workload by not doing transaction commit for every
subvolume creation, and merely requiring a transaction commit on fsync.
In the worst case, of doing a subvolume create and fsync in a loop, this
should require an equal amount of time to the current scheme; and in the
best case, the internal workload creating hundreds of subvols before
fsyncing is greatly improved.

While it would be nice to be able to use the log tree and use the normal
fsync path, logtree replay can't deal with new subvolume inodes
presently.

It's possible that there's some reason that the transaction commit is
necessary for correctness during subvolume creation; however,
git logs indicate that the commit dates back to the beginning of
subvolume creation, and there are no notes on why it would be necessary.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/btrfs/ioctl.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 25833b4eeaf5..a6f1ee2dc1b9 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -647,6 +647,8 @@ static noinline int create_subvol(struct mnt_idmap *idmap,
 	}
 	trans->block_rsv = &block_rsv;
 	trans->bytes_reserved = block_rsv.size;
+	/* tree log can't currently deal with an inode which is a new root */
+	btrfs_set_log_full_commit(trans);
 
 	ret = btrfs_qgroup_inherit(trans, 0, objectid, inherit);
 	if (ret)
@@ -755,10 +757,7 @@ static noinline int create_subvol(struct mnt_idmap *idmap,
 	trans->bytes_reserved = 0;
 	btrfs_subvolume_release_metadata(root, &block_rsv);
 
-	if (ret)
-		btrfs_end_transaction(trans);
-	else
-		ret = btrfs_commit_transaction(trans);
+	btrfs_end_transaction(trans);
 out_new_inode_args:
 	btrfs_new_inode_args_destroy(&new_inode_args);
 out_inode:
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-04-21 15:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-11 19:10 [PATCH] btrfs: don't commit transaction for every subvol create Sweet Tea Dorminy
2023-04-12  1:45 ` Neal Gompa
2023-04-12  2:34   ` Qu Wenruo
2023-04-12  2:35     ` Qu Wenruo
2023-04-12 13:49   ` Josef Bacik
2023-04-12  2:29 ` Qu Wenruo
2023-04-12 13:51 ` Josef Bacik
2023-04-12 22:35 ` Neal Gompa
2023-04-17 18:46 ` David Sterba
2023-04-21 12:58   ` Sweet Tea Dorminy
2023-04-21 15:23     ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.