Re: [PATCH] btrfs: remove @atomic parameter from btrfs_buffer_uptodate()

[Filipe Manana <fdmanana@kernel.org>]

On Sun, Mar 15, 2026 at 9:40 PM Qu Wenruo <wqu@suse.com> wrote:
>
> That parameter is introduced by commit b9fab919b748 ("Btrfs: avoid

is -> was

> sleeping in verify_parent_transid while atomic").
> At that time we need to lock the extent buffer range inside the io tree,

need -> needed

> to avoid content changes thus it may sleep.

it may sleep -> it could sleep

Also missing a comma between "changes" and "thus".

>
> But that behavior is no longer there, as later commit 947a629988f1
> ("btrfs: move tree block parentness check into
> validate_extent_buffer()") changed the transid check into the endio
> function thus there is no need to lock io tree.

Giving that commit as a reference, isn't very useful in my opinion.

It would be best to mention which commit removed the io tree locking
inside btrfs_buffer_uptodate(), which is:

9e2aff90fc2a ("btrfs: stop using lock_extent in btrfs_buffer_uptodate")

As that's what really matters and makes the function don't block/sleep anymore.

With that fixed:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

>
> We can remove the @atomic parameter safely now.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/ctree.c       | 2 +-
>  fs/btrfs/disk-io.c     | 7 ++-----
>  fs/btrfs/disk-io.h     | 2 +-
>  fs/btrfs/extent-tree.c | 2 +-
>  fs/btrfs/extent_io.c   | 6 +++---
>  fs/btrfs/tree-log.c    | 2 +-
>  6 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index cdd6c1422b53..5c68c541de51 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1499,7 +1499,7 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,
>                         reada_for_search(fs_info, p, parent_level, slot, key->objectid);
>
>                 /* first we do an atomic uptodate check */
> -               if (btrfs_buffer_uptodate(tmp, check.transid, true, NULL) > 0) {
> +               if (btrfs_buffer_uptodate(tmp, check.transid, NULL) > 0) {
>                         /*
>                          * Do extra check for first_key, eb can be stale due to
>                          * being cached, read from scrub, or have multiple
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 868afeb37235..75d8ab39601d 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -109,7 +109,7 @@ static void csum_tree_block(struct extent_buffer *buf, u8 *result)
>   * detect blocks that either didn't get written at all or got written
>   * in the wrong place.
>   */
> -int btrfs_buffer_uptodate(struct extent_buffer *eb, u64 parent_transid, bool atomic,
> +int btrfs_buffer_uptodate(struct extent_buffer *eb, u64 parent_transid,
>                           const struct btrfs_tree_parent_check *check)
>  {
>         if (!extent_buffer_uptodate(eb))
> @@ -125,9 +125,6 @@ int btrfs_buffer_uptodate(struct extent_buffer *eb, u64 parent_transid, bool ato
>                 return 1;
>         }
>
> -       if (atomic)
> -               return -EAGAIN;
> -
>         if (btrfs_header_generation(eb) != parent_transid) {
>                 btrfs_err_rl(eb->fs_info,
>  "parent transid verify failed on logical %llu mirror %u wanted %llu found %llu",
> @@ -1001,7 +998,7 @@ static struct btrfs_root *read_tree_root_path(struct btrfs_root *tree_root,
>                 goto fail;
>         }
>
> -       ret = btrfs_buffer_uptodate(root->node, generation, false, &check);
> +       ret = btrfs_buffer_uptodate(root->node, generation, &check);
>         if (unlikely(ret <= 0)) {
>                 if (ret == 0)
>                         ret = -EIO;
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 343e332b17c0..9185f8f02eeb 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -107,7 +107,7 @@ static inline struct btrfs_root *btrfs_grab_root(struct btrfs_root *root)
>  void btrfs_put_root(struct btrfs_root *root);
>  void btrfs_mark_buffer_dirty(struct btrfs_trans_handle *trans,
>                              struct extent_buffer *buf);
> -int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid, bool atomic,
> +int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,
>                           const struct btrfs_tree_parent_check *check);
>  int btrfs_read_extent_buffer(struct extent_buffer *buf,
>                              const struct btrfs_tree_parent_check *check);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index a0a0212c5f40..a55b910c755b 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -5780,7 +5780,7 @@ static int check_next_block_uptodate(struct btrfs_trans_handle *trans,
>
>         generation = btrfs_node_ptr_generation(path->nodes[level], path->slots[level]);
>
> -       if (btrfs_buffer_uptodate(next, generation, false, NULL))
> +       if (btrfs_buffer_uptodate(next, generation, NULL))
>                 return 0;
>
>         check.level = level - 1;
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 980f16313e11..56a2c8e01b83 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3905,7 +3905,7 @@ int read_extent_buffer_pages_nowait(struct extent_buffer *eb, int mirror_num,
>         if (extent_buffer_uptodate(eb)) {
>                 int ret;
>
> -               ret = btrfs_buffer_uptodate(eb, 0, true, check);
> +               ret = btrfs_buffer_uptodate(eb, 0, check);
>                 if (unlikely(ret <= 0)) {
>                         if (ret == 0)
>                                 ret = -EIO;
> @@ -3936,7 +3936,7 @@ int read_extent_buffer_pages_nowait(struct extent_buffer *eb, int mirror_num,
>                 int ret;
>
>                 clear_extent_buffer_reading(eb);
> -               ret = btrfs_buffer_uptodate(eb, 0, true, check);
> +               ret = btrfs_buffer_uptodate(eb, 0, check);
>                 if (unlikely(ret <= 0)) {
>                         if (ret == 0)
>                                 ret = -EIO;
> @@ -4654,7 +4654,7 @@ void btrfs_readahead_tree_block(struct btrfs_fs_info *fs_info,
>         if (IS_ERR(eb))
>                 return;
>
> -       if (btrfs_buffer_uptodate(eb, gen, true, NULL)) {
> +       if (btrfs_buffer_uptodate(eb, gen, NULL)) {
>                 free_extent_buffer(eb);
>                 return;
>         }
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 6cf2828ba500..de4707b1227e 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -457,7 +457,7 @@ static int process_one_buffer(struct extent_buffer *eb,
>                         return ret;
>                 }
>
> -               if (btrfs_buffer_uptodate(eb, gen, false, NULL) && level == 0) {
> +               if (btrfs_buffer_uptodate(eb, gen, NULL) && level == 0) {
>                         ret = btrfs_exclude_logged_extents(eb);
>                         if (ret)
>                                 btrfs_abort_transaction(trans, ret);
> --
> 2.53.0
>
>