Re: [PATCH 6/8] btrfs: simplify update of last_dir_index_offset when logging a directory

[Filipe Manana <fdmanana@kernel.org>]

On Tue, Jan 10, 2023 at 3:20 PM <fdmanana@kernel.org> wrote:
>
> From: Filipe Manana <fdmanana@suse.com>
>
> When logging a directory, we always set the inode's last_dir_index_offset
> to the offset of the last dir index item we found. This is using an extra
> field in the log context structure, and it makes more sense to update it
> only after we insert dir index items, and we could directly update the
> inode's last_dir_index_offset field instead.
>
> So make this simpler by updating the inode's last_dir_index_offset only
> when we actually insert dir index keys in the log tree, and getting rid
> of the last_dir_item_offset field in the log context structure.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reported-by: David Arendt <admin@prnet.org>
Link: https://lore.kernel.org/linux-btrfs/ae169fc6-f504-28f0-a098-6fa6a4dfb612@leemhuis.info/
Reported-by: Maxim Mikityanskiy <maxtram95@gmail.com>
Link: https://lore.kernel.org/linux-btrfs/Y8voyTXdnPDz8xwY@mail.gmail.com/
Reported-by: Hunter Wardlaw <wardlawhunter@gmail.com>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1207231
CC: stable@vger.kernel.org # 6.1+

David, can you please add the following tags to the patch?
It happens to fix an issue those users reported, all on 6.1 only.

Thanks.

> ---
>  fs/btrfs/tree-log.c | 23 +++++++++++++++++------
>  fs/btrfs/tree-log.h |  2 --
>  2 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index d43261545264..58599189bd18 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3576,17 +3576,19 @@ static noinline int insert_dir_log_key(struct btrfs_trans_handle *trans,
>  }
>
>  static int flush_dir_items_batch(struct btrfs_trans_handle *trans,
> -                                struct btrfs_root *log,
> +                                struct btrfs_inode *inode,
>                                  struct extent_buffer *src,
>                                  struct btrfs_path *dst_path,
>                                  int start_slot,
>                                  int count)
>  {
> +       struct btrfs_root *log = inode->root->log_root;
>         char *ins_data = NULL;
>         struct btrfs_item_batch batch;
>         struct extent_buffer *dst;
>         unsigned long src_offset;
>         unsigned long dst_offset;
> +       u64 last_index;
>         struct btrfs_key key;
>         u32 item_size;
>         int ret;
> @@ -3644,6 +3646,19 @@ static int flush_dir_items_batch(struct btrfs_trans_handle *trans,
>         src_offset = btrfs_item_ptr_offset(src, start_slot + count - 1);
>         copy_extent_buffer(dst, src, dst_offset, src_offset, batch.total_data_size);
>         btrfs_release_path(dst_path);
> +
> +       last_index = batch.keys[count - 1].offset;
> +       ASSERT(last_index > inode->last_dir_index_offset);
> +
> +       /*
> +        * If for some unexpected reason the last item's index is not greater
> +        * than the last index we logged, warn and return an error to fallback
> +        * to a transaction commit.
> +        */
> +       if (WARN_ON(last_index <= inode->last_dir_index_offset))
> +               ret = -EUCLEAN;
> +       else
> +               inode->last_dir_index_offset = last_index;
>  out:
>         kfree(ins_data);
>
> @@ -3693,7 +3708,6 @@ static int process_dir_items_leaf(struct btrfs_trans_handle *trans,
>                 }
>
>                 di = btrfs_item_ptr(src, i, struct btrfs_dir_item);
> -               ctx->last_dir_item_offset = key.offset;
>
>                 /*
>                  * Skip ranges of items that consist only of dir item keys created
> @@ -3756,7 +3770,7 @@ static int process_dir_items_leaf(struct btrfs_trans_handle *trans,
>         if (batch_size > 0) {
>                 int ret;
>
> -               ret = flush_dir_items_batch(trans, log, src, dst_path,
> +               ret = flush_dir_items_batch(trans, inode, src, dst_path,
>                                             batch_start, batch_size);
>                 if (ret < 0)
>                         return ret;
> @@ -4044,7 +4058,6 @@ static noinline int log_directory_changes(struct btrfs_trans_handle *trans,
>
>         min_key = BTRFS_DIR_START_INDEX;
>         max_key = 0;
> -       ctx->last_dir_item_offset = inode->last_dir_index_offset;
>
>         while (1) {
>                 ret = log_dir_items(trans, inode, path, dst_path,
> @@ -4056,8 +4069,6 @@ static noinline int log_directory_changes(struct btrfs_trans_handle *trans,
>                 min_key = max_key + 1;
>         }
>
> -       inode->last_dir_index_offset = ctx->last_dir_item_offset;
> -
>         return 0;
>  }
>
> diff --git a/fs/btrfs/tree-log.h b/fs/btrfs/tree-log.h
> index 85b43075ac58..85cd24cb0540 100644
> --- a/fs/btrfs/tree-log.h
> +++ b/fs/btrfs/tree-log.h
> @@ -24,8 +24,6 @@ struct btrfs_log_ctx {
>         bool logging_new_delayed_dentries;
>         /* Indicate if the inode being logged was logged before. */
>         bool logged_before;
> -       /* Tracks the last logged dir item/index key offset. */
> -       u64 last_dir_item_offset;
>         struct inode *inode;
>         struct list_head list;
>         /* Only used for fast fsyncs. */
> --
> 2.35.1
>