[PATCH v2 0/3] btrfs: avoid unnecessary transaction commit and tree-checker updates

[PATCH v2 0/3] btrfs: avoid unnecessary transaction commit and tree-checker updates

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Details in the change logs.

V2: Updated patch 1 due to sashiko's complain about lacking
    checks in tree-checker for inode ref key offsets and unnecessarily
    trigger assert failure instead of falling back to transaction commit.

    Added tree-checker patches to validate inode ref keys and inode extref items.

Filipe Manana (3):
  btrfs: fix unnecessary transaction commit fallback from btrfs_log_all_parents()
  btrfs: tree-checker: validate key offset for inode ref keys
  btrfs: tree-checker: validate parent field for inode extref items

 fs/btrfs/tree-checker.c | 22 ++++++++++++++++++++++
 fs/btrfs/tree-log.c     | 16 ++++++++++++++++
 2 files changed, 38 insertions(+)

-- 
2.47.2

[PATCH 1/3] btrfs: fix unnecessary transaction commit fallback from btrfs_log_all_parents()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When btrfs_log_all_parents() returns without doing any work (because all
parent directories were already logged), it returns 1, which is propagated
up the fsync call chain up to btrfs_log_dentry_safe(), and that causes
btrfs_sync_file() to trigger am unnecessary transaction commit.

This all happens because the call to btrfs_search_slot() in
btrfs_log_all_parents() always returns 1, as there can not be any inode
ref keys with an offset 0 (an invalid inode number), so if the while loop
below it does not do any work because all parent directories were already
logged, the 'ret' variable remains with a value of 1, which is then
returned up the call chain to btrfs_sync_file().

Fix this by setting 'ret' to 0 after the call to btrfs_search_slot().

Fixes: 0f24ea456ae1 ("btrfs: tracepoints: add trace event for btrfs_log_all_parents()")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 7ba7b6098aa5..a00094604e54 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -7286,6 +7286,22 @@ static int btrfs_log_all_parents(struct btrfs_trans_handle *trans,
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
 	if (ret < 0)
 		goto out;
+	/*
+	 * There can't be an inode ref key with offset 0 because inode numbers
+	 * start at BTRFS_FIRST_FREE_OBJECTID.
+	 */
+	if (WARN_ON_ONCE(ret == 0)) {
+		btrfs_err(trans->fs_info,
+		  "found inode ref key with offset 0 for root %llu inode %llu",
+			  btrfs_root_id(root), ino);
+		ret = BTRFS_LOG_FORCE_COMMIT;
+		goto out;
+	}
+	/*
+	 * Set to 0 so that in case we don't do any work below, we won't return
+	 * 1 and trigger an unnecessary transaction commit.
+	 */
+	ret = 0;
 
 	while (true) {
 		struct extent_buffer *leaf = path->nodes[0];
-- 
2.47.2

[PATCH 2/3] btrfs: tree-checker: validate key offset for inode ref keys

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

For a subvolume tree, the offset of an inode ref key corresponds to an
inode number, and that must always be within the range:

  [ BTRFS_FIRST_FREE_OBJECTID (256), BTRFS_LAST_FREE_OBJECTID (-256) ]

Add a check for that in check_inode_ref(). Sashiko had complained about
such check missing in another unrelated patch.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-checker.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 0ce91396b517..baddde54dfab 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -1909,6 +1909,16 @@ static int check_inode_ref(struct extent_buffer *leaf,
 		return -EUCLEAN;
 	}
 
+	if (unlikely(btrfs_is_fstree(btrfs_header_owner(leaf) &&
+	     (key->offset < BTRFS_FIRST_FREE_OBJECTID ||
+	      key->offset > BTRFS_LAST_FREE_OBJECTID)))) {
+		inode_ref_err(leaf, slot,
+			      "invalid offset for ref key, have %llu expect [%llu, %lld]",
+			      key->offset, BTRFS_FIRST_FREE_OBJECTID,
+			      BTRFS_LAST_FREE_OBJECTID);
+		return -EUCLEAN;
+	}
+
 	ptr = btrfs_item_ptr_offset(leaf, slot);
 	end = ptr + btrfs_item_size(leaf, slot);
 	while (ptr < end) {
-- 
2.47.2

[PATCH 3/3] btrfs: tree-checker: validate parent field for inode extref items

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

For a subvolume tree, the parent field of an inode extref item corresponds
to an inode number, and that must always be within the range:

   [ BTRFS_FIRST_FREE_OBJECTID (256), BTRFS_LAST_FREE_OBJECTID (-256) ]

Add a check for that in check_inode_extref().

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-checker.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index baddde54dfab..9dabae134e93 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -1962,12 +1962,14 @@ static int check_inode_extref(struct extent_buffer *leaf,
 {
 	unsigned long ptr = btrfs_item_ptr_offset(leaf, slot);
 	unsigned long end = ptr + btrfs_item_size(leaf, slot);
+	const bool is_fstree = btrfs_is_fstree(btrfs_header_owner(leaf));
 
 	if (unlikely(!check_prev_ino(leaf, key, slot, prev_key)))
 		return -EUCLEAN;
 
 	while (ptr < end) {
 		struct btrfs_inode_extref *extref = (struct btrfs_inode_extref *)ptr;
+		u64 parent;
 		u16 namelen;
 
 		if (unlikely(ptr + sizeof(*extref) > end)) {
@@ -1977,6 +1979,16 @@ static int check_inode_extref(struct extent_buffer *leaf,
 			return -EUCLEAN;
 		}
 
+		parent = btrfs_inode_extref_parent(leaf, extref);
+		if (unlikely(is_fstree && (parent < BTRFS_FIRST_FREE_OBJECTID ||
+					   parent > BTRFS_LAST_FREE_OBJECTID))) {
+			inode_ref_err(leaf, slot,
+		      "invalid parent for extref key, have %llu expect [%llu, %lld]",
+			      parent, BTRFS_FIRST_FREE_OBJECTID,
+				      BTRFS_LAST_FREE_OBJECTID);
+			return -EUCLEAN;
+		}
+
 		namelen = btrfs_inode_extref_name_len(leaf, extref);
 		if (unlikely(ptr + sizeof(*extref) + namelen > end)) {
 			inode_ref_err(leaf, slot,
-- 
2.47.2

[PATCH v3 0/3] btrfs: avoid unnecessary transaction commit and tree-checker updates

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Details in the change logs.

V3: Fix misplaced parenthesis in patch 2/3, making a condition always
    evaluate to false.

V2: Updated patch 1 due to sashiko's complain about lacking
    checks in tree-checker for inode ref key offsets and unnecessarily
    trigger assert failure instead of falling back to transaction commit.

    Added tree-checker patches to validate inode ref keys and inode extref items.

Filipe Manana (3):
  btrfs: fix unnecessary transaction commit fallback from btrfs_log_all_parents()
  btrfs: tree-checker: validate key offset for inode ref keys
  btrfs: tree-checker: validate parent field for inode extref items

 fs/btrfs/tree-checker.c | 22 ++++++++++++++++++++++
 fs/btrfs/tree-log.c     | 16 ++++++++++++++++
 2 files changed, 38 insertions(+)

-- 
2.47.2

[PATCH v3 1/3] btrfs: fix unnecessary transaction commit fallback from btrfs_log_all_parents()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When btrfs_log_all_parents() returns without doing any work (because all
parent directories were already logged), it returns 1, which is propagated
up the fsync call chain up to btrfs_log_dentry_safe(), and that causes
btrfs_sync_file() to trigger am unnecessary transaction commit.

This all happens because the call to btrfs_search_slot() in
btrfs_log_all_parents() always returns 1, as there can not be any inode
ref keys with an offset 0 (an invalid inode number), so if the while loop
below it does not do any work because all parent directories were already
logged, the 'ret' variable remains with a value of 1, which is then
returned up the call chain to btrfs_sync_file().

Fix this by setting 'ret' to 0 after the call to btrfs_search_slot().

Fixes: 0f24ea456ae1 ("btrfs: tracepoints: add trace event for btrfs_log_all_parents()")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 7ba7b6098aa5..a00094604e54 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -7286,6 +7286,22 @@ static int btrfs_log_all_parents(struct btrfs_trans_handle *trans,
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
 	if (ret < 0)
 		goto out;
+	/*
+	 * There can't be an inode ref key with offset 0 because inode numbers
+	 * start at BTRFS_FIRST_FREE_OBJECTID.
+	 */
+	if (WARN_ON_ONCE(ret == 0)) {
+		btrfs_err(trans->fs_info,
+		  "found inode ref key with offset 0 for root %llu inode %llu",
+			  btrfs_root_id(root), ino);
+		ret = BTRFS_LOG_FORCE_COMMIT;
+		goto out;
+	}
+	/*
+	 * Set to 0 so that in case we don't do any work below, we won't return
+	 * 1 and trigger an unnecessary transaction commit.
+	 */
+	ret = 0;
 
 	while (true) {
 		struct extent_buffer *leaf = path->nodes[0];
-- 
2.47.2

[PATCH v3 2/3] btrfs: tree-checker: validate key offset for inode ref keys

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

For a subvolume tree, the offset of an inode ref key corresponds to an
inode number, and that must always be within the range:

  [ BTRFS_FIRST_FREE_OBJECTID (256), BTRFS_LAST_FREE_OBJECTID (-256) ]

Add a check for that in check_inode_ref(). Sashiko had complained about
such check missing in another unrelated patch.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-checker.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 622334ffd240..7dff824cfc4c 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -1951,6 +1951,16 @@ static int check_inode_ref(struct extent_buffer *leaf,
 		return -EUCLEAN;
 	}
 
+	if (unlikely(btrfs_is_fstree(btrfs_header_owner(leaf)) &&
+	     (key->offset < BTRFS_FIRST_FREE_OBJECTID ||
+	      key->offset > BTRFS_LAST_FREE_OBJECTID))) {
+		inode_ref_err(leaf, slot,
+			      "invalid offset for ref key, have %llu expect [%llu, %lld]",
+			      key->offset, BTRFS_FIRST_FREE_OBJECTID,
+			      BTRFS_LAST_FREE_OBJECTID);
+		return -EUCLEAN;
+	}
+
 	ptr = btrfs_item_ptr_offset(leaf, slot);
 	end = ptr + btrfs_item_size(leaf, slot);
 	while (ptr < end) {
-- 
2.47.2

[PATCH v3 3/3] btrfs: tree-checker: validate parent field for inode extref items

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

For a subvolume tree, the parent field of an inode extref item corresponds
to an inode number, and that must always be within the range:

   [ BTRFS_FIRST_FREE_OBJECTID (256), BTRFS_LAST_FREE_OBJECTID (-256) ]

Add a check for that in check_inode_extref().

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-checker.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 7dff824cfc4c..3a0715798282 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -2004,12 +2004,14 @@ static int check_inode_extref(struct extent_buffer *leaf,
 {
 	unsigned long ptr = btrfs_item_ptr_offset(leaf, slot);
 	unsigned long end = ptr + btrfs_item_size(leaf, slot);
+	const bool is_fstree = btrfs_is_fstree(btrfs_header_owner(leaf));
 
 	if (unlikely(!check_prev_ino(leaf, key, slot, prev_key)))
 		return -EUCLEAN;
 
 	while (ptr < end) {
 		struct btrfs_inode_extref *extref = (struct btrfs_inode_extref *)ptr;
+		u64 parent;
 		u16 namelen;
 
 		if (unlikely(ptr + sizeof(*extref) > end)) {
@@ -2019,6 +2021,16 @@ static int check_inode_extref(struct extent_buffer *leaf,
 			return -EUCLEAN;
 		}
 
+		parent = btrfs_inode_extref_parent(leaf, extref);
+		if (unlikely(is_fstree && (parent < BTRFS_FIRST_FREE_OBJECTID ||
+					   parent > BTRFS_LAST_FREE_OBJECTID))) {
+			inode_ref_err(leaf, slot,
+		      "invalid parent for extref key, have %llu expect [%llu, %lld]",
+			      parent, BTRFS_FIRST_FREE_OBJECTID,
+				      BTRFS_LAST_FREE_OBJECTID);
+			return -EUCLEAN;
+		}
+
 		namelen = btrfs_inode_extref_name_len(leaf, extref);
 		if (unlikely(ptr + sizeof(*extref) + namelen > end)) {
 			inode_ref_err(leaf, slot,
-- 
2.47.2

Re: [PATCH v3 0/3] btrfs: avoid unnecessary transaction commit and tree-checker updates

[Boris Burkov]

On Thu, Sep 03, 2026 at 05:00:30PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Details in the change logs.

Sorry for the redundant review, I somehow also got your previous standalone
version in my filtered email set for reviews.

I like the WARN version as well, I briefly considered asking for that
but figured ASSERT got the point across anyway :)

Reviewed-by: Boris Burkov <boris@bur.io>

Thanks

> 
> V3: Fix misplaced parenthesis in patch 2/3, making a condition always
>     evaluate to false.
> 
> V2: Updated patch 1 due to sashiko's complain about lacking
>     checks in tree-checker for inode ref key offsets and unnecessarily
>     trigger assert failure instead of falling back to transaction commit.
> 
>     Added tree-checker patches to validate inode ref keys and inode extref items.
> 
> Filipe Manana (3):
>   btrfs: fix unnecessary transaction commit fallback from btrfs_log_all_parents()
>   btrfs: tree-checker: validate key offset for inode ref keys
>   btrfs: tree-checker: validate parent field for inode extref items
> 
>  fs/btrfs/tree-checker.c | 22 ++++++++++++++++++++++
>  fs/btrfs/tree-log.c     | 16 ++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> -- 
> 2.47.2
> 

Re: [PATCH v3 0/3] btrfs: avoid unnecessary transaction commit and tree-checker updates

[Qu Wenruo]

在 2026/9/4 01:30, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Details in the change logs.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> 
> V3: Fix misplaced parenthesis in patch 2/3, making a condition always
>      evaluate to false.
> 
> V2: Updated patch 1 due to sashiko's complain about lacking
>      checks in tree-checker for inode ref key offsets and unnecessarily
>      trigger assert failure instead of falling back to transaction commit.
> 
>      Added tree-checker patches to validate inode ref keys and inode extref items.
> 
> Filipe Manana (3):
>    btrfs: fix unnecessary transaction commit fallback from btrfs_log_all_parents()
>    btrfs: tree-checker: validate key offset for inode ref keys
>    btrfs: tree-checker: validate parent field for inode extref items
> 
>   fs/btrfs/tree-checker.c | 22 ++++++++++++++++++++++
>   fs/btrfs/tree-log.c     | 16 ++++++++++++++++
>   2 files changed, 38 insertions(+)
>