* [PATCH] btrfs: remove @atomic parameter from btrfs_buffer_uptodate()
@ 2026-03-15 21:38 Qu Wenruo
2026-03-16 16:07 ` Filipe Manana
0 siblings, 1 reply; 2+ messages in thread
From: Qu Wenruo @ 2026-03-15 21:38 UTC (permalink / raw)
To: linux-btrfs
That parameter is introduced by commit b9fab919b748 ("Btrfs: avoid
sleeping in verify_parent_transid while atomic").
At that time we need to lock the extent buffer range inside the io tree,
to avoid content changes thus it may sleep.
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.
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] btrfs: remove @atomic parameter from btrfs_buffer_uptodate()
2026-03-15 21:38 [PATCH] btrfs: remove @atomic parameter from btrfs_buffer_uptodate() Qu Wenruo
@ 2026-03-16 16:07 ` Filipe Manana
0 siblings, 0 replies; 2+ messages in thread
From: Filipe Manana @ 2026-03-16 16:07 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
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
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-03-16 16:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-15 21:38 [PATCH] btrfs: remove @atomic parameter from btrfs_buffer_uptodate() Qu Wenruo
2026-03-16 16:07 ` Filipe Manana
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox