On Thu, Feb 4, 2021 at 10:23 AM Naohiro Aota <naohiro.aota@xxxxxxx> wrote:
>
> This is the 2/3 patch to enable tree-log on zoned filesystems.
>
> Since we can start more than one log transactions per subvolume
> simultaneously, nodes from multiple transactions can be allocated
> interleaved. Such mixed allocation results in non-sequential writes at the
> time of a log transaction commit. The nodes of the global log root tree
> (fs_info->log_root_tree), also have the same problem with mixed
> allocation.
>
> Serializes log transactions by waiting for a committing transaction when
> someone tries to start a new transaction, to avoid the mixed allocation
> problem. We must also wait for running log transactions from another
> subvolume, but there is no easy way to detect which subvolume root is
> running a log transaction. So, this patch forbids starting a new log
> transaction when other subvolumes already allocated the global log root
> tree.
>
> Cc: Filipe Manana <fdmanana@xxxxxxxxx>
> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Signed-off-by: Naohiro Aota <naohiro.aota@xxxxxxx>
> ---
> fs/btrfs/tree-log.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index c02eeeac439c..8be3164d4c5d 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -105,6 +105,7 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,
> struct btrfs_root *log,
> struct btrfs_path *path,
> u64 dirid, int del_all);
> +static void wait_log_commit(struct btrfs_root *root, int transid);
>
> /*
> * tree logging is a special write ahead log used to make sure that
> @@ -140,6 +141,7 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
> {
> struct btrfs_fs_info *fs_info = root->fs_info;
> struct btrfs_root *tree_root = fs_info->tree_root;
> + const bool zoned = btrfs_is_zoned(fs_info);
> int ret = 0;
>
> /*
> @@ -160,12 +162,20 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
>
> mutex_lock(&root->log_mutex);
>
> +again:
> if (root->log_root) {
> + int index = (root->log_transid + 1) % 2;
> +
> if (btrfs_need_log_full_commit(trans)) {
> ret = -EAGAIN;
> goto out;
> }
>
> + if (zoned && atomic_read(&root->log_commit[index])) {
> + wait_log_commit(root, root->log_transid - 1);
> + goto again;
> + }
> +
> if (!root->log_start_pid) {
> clear_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state);
> root->log_start_pid = current->pid;
> @@ -173,6 +183,17 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
> set_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state);
> }
> } else {
> + if (zoned) {
> + mutex_lock(&fs_info->tree_log_mutex);
> + if (fs_info->log_root_tree)
> + ret = -EAGAIN;
> + else
> + ret = btrfs_init_log_root_tree(trans, fs_info);
> + mutex_unlock(&fs_info->tree_log_mutex);
> + }
So, nothing here changed since v14 - all my comments still apply [1]
This is based on pre-5.10 code and is broken as it is - it results in
every fsync falling back to a transaction commit, defeating the
purpose of all the patches that deal with log trees on zoned
filesystems.
Thanks.
[1] https://lore.kernel.org/linux-btrfs/CAL3q7H5pv416FVwThOHe+M3L5B-z_n6_ZGQQxsUq5vC5fsAoJw@xxxxxxxxxxxxxx/
> + if (ret)
> + goto out;
> +
> ret = btrfs_add_log_tree(trans, root);
> if (ret)
> goto out;
> @@ -201,14 +222,22 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
> */
> static int join_running_log_trans(struct btrfs_root *root)
> {
> + const bool zoned = btrfs_is_zoned(root->fs_info);
> int ret = -ENOENT;
>
> if (!test_bit(BTRFS_ROOT_HAS_LOG_TREE, &root->state))
> return ret;
>
> mutex_lock(&root->log_mutex);
> +again:
> if (root->log_root) {
> + int index = (root->log_transid + 1) % 2;
> +
> ret = 0;
> + if (zoned && atomic_read(&root->log_commit[index])) {
> + wait_log_commit(root, root->log_transid - 1);
> + goto again;
> + }
> atomic_inc(&root->log_writers);
> }
> mutex_unlock(&root->log_mutex);
> --
> 2.30.0
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
