[PATCH v2 0/9] extent buffer dirty cleanups

[PATCH v2 0/9] extent buffer dirty cleanups

[Josef Bacik]

v1->v2:
- Added "btrfs: do not call btrfs_clean_tree_block in update_ref_for_cow", Qu
  noticed some corruption with the original patchset, this turned out to be
  because we were clearing the extent buffer dirty at cow time, which doesn't
  make sense as we're not free'ing the current block in our current transaction.

--- Original email ---
Hello,

While sync'ing ctree.c to btrfs-progs I noticed we have some oddities when it
comes to how we deal with the extent buffer being dirty.  We have
btrfs_clean_tree_block, which is sort of meant to be run against extent buffers
we've modified in this transaction.  However we have some other places where
we've open coded the same work without the generation check.  This makes it kind
of confusing, and is inconsistent with how we deal with the
fs_info->dirty_metadata_bytes.

So clean this stuff up so we have one helper we use for setting the extent
buffer dirty (btrfs_mark_buffer_dirty) and one for clearing dirty
(btrfs_clear_buffer_dirty).  This makes everything more consistent and clean
across the board.  I've additionally cleaned up a random writeback thing we had
in tree-log that I noticed while doing these cleanups.  Thanks,

Josef

Josef Bacik (9):
  btrfs: do not call btrfs_clean_tree_block in update_ref_for_cow
  btrfs: always lock the block before calling btrfs_clean_tree_block
  btrfs: do not check header generation in btrfs_clean_tree_block
  btrfs: do not set the header generation before btrfs_clean_tree_block
  btrfs: replace clearing extent buffer dirty bit with btrfs_clean_block
  btrfs: do not increment dirty_metadata_bytes in set_btree_ioerr
  btrfs: rename btrfs_clean_tree_block => btrfs_clear_buffer_dirty
  btrfs: combine btrfs_clear_buffer_dirty and clear_extent_buffer_dirty
  btrfs: remove btrfs_wait_tree_block_writeback

 fs/btrfs/ctree.c           | 15 +++++++-------
 fs/btrfs/disk-io.c         | 25 +++++-------------------
 fs/btrfs/disk-io.h         |  2 +-
 fs/btrfs/extent-tree.c     | 12 ++++--------
 fs/btrfs/extent_io.c       | 18 +++++++++--------
 fs/btrfs/extent_io.h       |  2 +-
 fs/btrfs/free-space-tree.c |  2 +-
 fs/btrfs/ioctl.c           |  2 +-
 fs/btrfs/qgroup.c          |  2 +-
 fs/btrfs/tree-log.c        | 40 ++++++++++++++------------------------
 10 files changed, 46 insertions(+), 74 deletions(-)

-- 
2.26.3

[PATCH v2 1/9] btrfs: do not call btrfs_clean_tree_block in update_ref_for_cow

[Josef Bacik]

My series cleaning up the btrfs_clean_tree_block usage uncovered a
pretty nasty problem, we weren't writing out metadata that was getting
cow'ed away before the transaction commit was able to write out the
block.

This was because I changed btrfs_clean_tree_block() to unconditionally
clear the EXTENT_BUFFER_DIRTY flag, whereas before it was only doing it
if the header generation == the transid of the currently running
transaction.  This check exists purely for update_ref_for_cow's benefit,
as we only really want to call this when we're sure we won't use the
block again.  In the case of update_ref_for_cow() we're only truly
wanting to avoid using the block if we're in the same transaction that
dirtied it originally.  However we only get into update_ref_for_cow()
with refs == 1 if we already wrote the block to disk via memory pressure
or something, which means we won't have EXTENT_BUFFER_DIRTY set anyway.

Remove this usage in update_ref_for_cow(), as it doesn't make sense, and
is actually harmful with my other cleanups.  We want to reserve the use
of this function for when we free a block in the current transaction,
like with delete's or balances.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 5476d90a76ce..ac33dbb08263 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -459,7 +459,6 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
 			if (ret)
 				return ret;
 		}
-		btrfs_clean_tree_block(buf);
 		*last_ref = 1;
 	}
 	return 0;
-- 
2.26.3

[PATCH v2 2/9] btrfs: always lock the block before calling btrfs_clean_tree_block

[Josef Bacik]

We want to clean up the dirty handling for extent buffers so it's a
little more consistent, so skip the check for generation == transid and
simply always lock the extent buffer before calling
btrfs_clean_tree_block.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/extent-tree.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 876bea67f9a1..c85af644e353 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -5534,8 +5534,7 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,
 			}
 		}
 		/* make block locked assertion in btrfs_clean_tree_block happy */
-		if (!path->locks[level] &&
-		    btrfs_header_generation(eb) == trans->transid) {
+		if (!path->locks[level]) {
 			btrfs_tree_lock(eb);
 			path->locks[level] = BTRFS_WRITE_LOCK;
 		}
-- 
2.26.3

[PATCH v2 3/9] btrfs: do not check header generation in btrfs_clean_tree_block

[Josef Bacik]

This check is from an era where we didn't have a per-extent buffer dirty
flag, we just messed with the page bits.  All the places we call this
function are when we have a transaction open, so just skip this check
and rely on the dirty flag.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/disk-io.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index d0ed52cab304..267163e546a5 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -968,16 +968,13 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
 void btrfs_clean_tree_block(struct extent_buffer *buf)
 {
 	struct btrfs_fs_info *fs_info = buf->fs_info;
-	if (btrfs_header_generation(buf) ==
-	    fs_info->running_transaction->transid) {
-		btrfs_assert_tree_write_locked(buf);
+	btrfs_assert_tree_write_locked(buf);
 
-		if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &buf->bflags)) {
-			percpu_counter_add_batch(&fs_info->dirty_metadata_bytes,
-						 -buf->len,
-						 fs_info->dirty_metadata_batch);
-			clear_extent_buffer_dirty(buf);
-		}
+	if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &buf->bflags)) {
+		percpu_counter_add_batch(&fs_info->dirty_metadata_bytes,
+					 -buf->len,
+					 fs_info->dirty_metadata_batch);
+		clear_extent_buffer_dirty(buf);
 	}
 }
 
-- 
2.26.3

[PATCH v2 4/9] btrfs: do not set the header generation before btrfs_clean_tree_block

[Josef Bacik]

We were doing this before to deal with possible uninitialized pages
which was reported by syzbot.  Now that btrfs_clean_tree_block no longer
checks the header generation simply remove this extra init.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/extent-tree.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index c85af644e353..971b1de50d9e 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4897,9 +4897,6 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	    !test_bit(BTRFS_ROOT_RESET_LOCKDEP_CLASS, &root->state))
 		lockdep_owner = BTRFS_FS_TREE_OBJECTID;
 
-	/* btrfs_clean_tree_block() accesses generation field. */
-	btrfs_set_header_generation(buf, trans->transid);
-
 	/*
 	 * This needs to stay, because we could allocate a freed block from an
 	 * old tree into a new tree, so we need to make sure this new block is
-- 
2.26.3

[PATCH v2 5/9] btrfs: replace clearing extent buffer dirty bit with btrfs_clean_block

[Josef Bacik]

Now that btrfs_clean_block doesn't care about the transid, we can
replace all occurrences of

if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &eb->bflags))
	clear_extent_buffer_dirty(eb);

with

btrfs_tree_lock(eb);
btrfs_clean_tree_block(eb);
btrfs_tree_unlock(eb);

We need the lock because if we are actually dirty we need to make sure
we aren't racing with anything that's starting writeout currently.  This
also makes sure that we're accounting fs_info->dirty_metadata_bytes
appropriately.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/disk-io.c  |  9 +++++----
 fs/btrfs/tree-log.c | 34 +++++++++++++++-------------------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 267163e546a5..275ba1925eeb 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -5074,11 +5074,12 @@ static int btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
 			start += fs_info->nodesize;
 			if (!eb)
 				continue;
-			wait_on_extent_buffer_writeback(eb);
 
-			if (test_and_clear_bit(EXTENT_BUFFER_DIRTY,
-					       &eb->bflags))
-				clear_extent_buffer_dirty(eb);
+			btrfs_tree_lock(eb);
+			wait_on_extent_buffer_writeback(eb);
+			btrfs_clean_tree_block(eb);
+			btrfs_tree_unlock(eb);
+
 			free_extent_buffer_stale(eb);
 		}
 	}
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 8fcfaf015a70..73e621df32f7 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2635,11 +2635,12 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
 					return ret;
 				}
 
+				btrfs_tree_lock(next);
+				btrfs_clean_tree_block(next);
+				btrfs_wait_tree_block_writeback(next);
+				btrfs_tree_unlock(next);
+
 				if (trans) {
-					btrfs_tree_lock(next);
-					btrfs_clean_tree_block(next);
-					btrfs_wait_tree_block_writeback(next);
-					btrfs_tree_unlock(next);
 					ret = btrfs_pin_reserved_extent(trans,
 							bytenr, blocksize);
 					if (ret) {
@@ -2649,8 +2650,6 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
 					btrfs_redirty_list_add(
 						trans->transaction, next);
 				} else {
-					if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &next->bflags))
-						clear_extent_buffer_dirty(next);
 					unaccount_log_buffer(fs_info, bytenr);
 				}
 			}
@@ -2705,11 +2704,12 @@ static noinline int walk_up_log_tree(struct btrfs_trans_handle *trans,
 
 				next = path->nodes[*level];
 
+				btrfs_tree_lock(next);
+				btrfs_clean_tree_block(next);
+				btrfs_wait_tree_block_writeback(next);
+				btrfs_tree_unlock(next);
+
 				if (trans) {
-					btrfs_tree_lock(next);
-					btrfs_clean_tree_block(next);
-					btrfs_wait_tree_block_writeback(next);
-					btrfs_tree_unlock(next);
 					ret = btrfs_pin_reserved_extent(trans,
 						     path->nodes[*level]->start,
 						     path->nodes[*level]->len);
@@ -2718,9 +2718,6 @@ static noinline int walk_up_log_tree(struct btrfs_trans_handle *trans,
 					btrfs_redirty_list_add(trans->transaction,
 							       next);
 				} else {
-					if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &next->bflags))
-						clear_extent_buffer_dirty(next);
-
 					unaccount_log_buffer(fs_info,
 						path->nodes[*level]->start);
 				}
@@ -2788,19 +2785,18 @@ static int walk_log_tree(struct btrfs_trans_handle *trans,
 
 			next = path->nodes[orig_level];
 
+			btrfs_tree_lock(next);
+			btrfs_clean_tree_block(next);
+			btrfs_wait_tree_block_writeback(next);
+			btrfs_tree_unlock(next);
+
 			if (trans) {
-				btrfs_tree_lock(next);
-				btrfs_clean_tree_block(next);
-				btrfs_wait_tree_block_writeback(next);
-				btrfs_tree_unlock(next);
 				ret = btrfs_pin_reserved_extent(trans,
 						next->start, next->len);
 				if (ret)
 					goto out;
 				btrfs_redirty_list_add(trans->transaction, next);
 			} else {
-				if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &next->bflags))
-					clear_extent_buffer_dirty(next);
 				unaccount_log_buffer(fs_info, next->start);
 			}
 		}
-- 
2.26.3

[PATCH v2 6/9] btrfs: do not increment dirty_metadata_bytes in set_btree_ioerr

[Josef Bacik]

We only add if we set the extent buffer dirty, and we subtract when we
clear the extent buffer dirty.  If we end up in set_btree_ioerr we have
already cleared the buffer dirty, and we aren't resetting dirty on the
extent buffer, so this is simply wrong.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/extent_io.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index c87be46e0663..7cd4065acc82 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2467,13 +2467,6 @@ static void set_btree_ioerr(struct page *page, struct extent_buffer *eb)
 	 */
 	mapping_set_error(page->mapping, -EIO);
 
-	/*
-	 * If we error out, we should add back the dirty_metadata_bytes
-	 * to make it consistent.
-	 */
-	percpu_counter_add_batch(&fs_info->dirty_metadata_bytes,
-				 eb->len, fs_info->dirty_metadata_batch);
-
 	/*
 	 * If writeback for a btree extent that doesn't belong to a log tree
 	 * failed, increment the counter transaction->eb_write_errors.
-- 
2.26.3

[PATCH v2 7/9] btrfs: rename btrfs_clean_tree_block => btrfs_clear_buffer_dirty

[Josef Bacik]

btrfs_clean_tree_block is a misnomer, it's just
clear_extent_buffer_dirty with some extra accounting around it.  Rename
this to btrfs_clear_buffer_dirty to make it more clear it belongs with
it's setter, btrfs_mark_buffer_dirty.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.c           | 14 +++++++-------
 fs/btrfs/disk-io.c         |  4 ++--
 fs/btrfs/disk-io.h         |  2 +-
 fs/btrfs/extent-tree.c     |  6 +++---
 fs/btrfs/free-space-tree.c |  2 +-
 fs/btrfs/ioctl.c           |  2 +-
 fs/btrfs/qgroup.c          |  2 +-
 fs/btrfs/tree-log.c        |  6 +++---
 8 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index ac33dbb08263..0b551d38d365 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1025,7 +1025,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
 
 		path->locks[level] = 0;
 		path->nodes[level] = NULL;
-		btrfs_clean_tree_block(mid);
+		btrfs_clear_buffer_dirty(mid);
 		btrfs_tree_unlock(mid);
 		/* once for the path */
 		free_extent_buffer(mid);
@@ -1086,7 +1086,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
 		if (wret < 0 && wret != -ENOSPC)
 			ret = wret;
 		if (btrfs_header_nritems(right) == 0) {
-			btrfs_clean_tree_block(right);
+			btrfs_clear_buffer_dirty(right);
 			btrfs_tree_unlock(right);
 			del_ptr(root, path, level + 1, pslot + 1);
 			root_sub_used(root, right->len);
@@ -1132,7 +1132,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
 		BUG_ON(wret == 1);
 	}
 	if (btrfs_header_nritems(mid) == 0) {
-		btrfs_clean_tree_block(mid);
+		btrfs_clear_buffer_dirty(mid);
 		btrfs_tree_unlock(mid);
 		del_ptr(root, path, level + 1, pslot);
 		root_sub_used(root, mid->len);
@@ -3105,7 +3105,7 @@ static noinline int __push_leaf_right(struct btrfs_path *path,
 	if (left_nritems)
 		btrfs_mark_buffer_dirty(left);
 	else
-		btrfs_clean_tree_block(left);
+		btrfs_clear_buffer_dirty(left);
 
 	btrfs_mark_buffer_dirty(right);
 
@@ -3117,7 +3117,7 @@ static noinline int __push_leaf_right(struct btrfs_path *path,
 	if (path->slots[0] >= left_nritems) {
 		path->slots[0] -= left_nritems;
 		if (btrfs_header_nritems(path->nodes[0]) == 0)
-			btrfs_clean_tree_block(path->nodes[0]);
+			btrfs_clear_buffer_dirty(path->nodes[0]);
 		btrfs_tree_unlock(path->nodes[0]);
 		free_extent_buffer(path->nodes[0]);
 		path->nodes[0] = right;
@@ -3329,7 +3329,7 @@ static noinline int __push_leaf_left(struct btrfs_path *path, int data_size,
 	if (right_nritems)
 		btrfs_mark_buffer_dirty(right);
 	else
-		btrfs_clean_tree_block(right);
+		btrfs_clear_buffer_dirty(right);
 
 	btrfs_item_key(right, &disk_key, 0);
 	fixup_low_keys(path, &disk_key, 1);
@@ -4366,7 +4366,7 @@ int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		if (leaf == root->node) {
 			btrfs_set_header_level(leaf, 0);
 		} else {
-			btrfs_clean_tree_block(leaf);
+			btrfs_clear_buffer_dirty(leaf);
 			btrfs_del_leaf(trans, root, path, leaf);
 		}
 	} else {
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 275ba1925eeb..b8f1ac54d10c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -965,7 +965,7 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
 
 }
 
-void btrfs_clean_tree_block(struct extent_buffer *buf)
+void btrfs_clear_buffer_dirty(struct extent_buffer *buf)
 {
 	struct btrfs_fs_info *fs_info = buf->fs_info;
 	btrfs_assert_tree_write_locked(buf);
@@ -5077,7 +5077,7 @@ static int btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
 
 			btrfs_tree_lock(eb);
 			wait_on_extent_buffer_writeback(eb);
-			btrfs_clean_tree_block(eb);
+			btrfs_clear_buffer_dirty(eb);
 			btrfs_tree_unlock(eb);
 
 			free_extent_buffer_stale(eb);
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index f2c507fd0e04..8d23746c7660 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -39,7 +39,7 @@ struct extent_buffer *btrfs_find_create_tree_block(
 						struct btrfs_fs_info *fs_info,
 						u64 bytenr, u64 owner_root,
 						int level);
-void btrfs_clean_tree_block(struct extent_buffer *buf);
+void btrfs_clear_buffer_dirty(struct extent_buffer *buf);
 void btrfs_clear_oneshot_options(struct btrfs_fs_info *fs_info);
 int btrfs_start_pre_rw_mount(struct btrfs_fs_info *fs_info);
 int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 971b1de50d9e..3f123c3570a9 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4905,7 +4905,7 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	btrfs_set_buffer_lockdep_class(lockdep_owner, buf, level);
 
 	__btrfs_tree_lock(buf, nest);
-	btrfs_clean_tree_block(buf);
+	btrfs_clear_buffer_dirty(buf);
 	clear_bit(EXTENT_BUFFER_STALE, &buf->bflags);
 	clear_bit(EXTENT_BUFFER_NO_CHECK, &buf->bflags);
 
@@ -5530,12 +5530,12 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,
 				}
 			}
 		}
-		/* make block locked assertion in btrfs_clean_tree_block happy */
+		/* make block locked assertion in btrfs_clear_buffer_dirty happy */
 		if (!path->locks[level]) {
 			btrfs_tree_lock(eb);
 			path->locks[level] = BTRFS_WRITE_LOCK;
 		}
-		btrfs_clean_tree_block(eb);
+		btrfs_clear_buffer_dirty(eb);
 	}
 
 	if (eb == root->node) {
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index c667e878ef1a..e03f58f43719 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1283,7 +1283,7 @@ int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info)
 	list_del(&free_space_root->dirty_list);
 
 	btrfs_tree_lock(free_space_root->node);
-	btrfs_clean_tree_block(free_space_root->node);
+	btrfs_clear_buffer_dirty(free_space_root->node);
 	btrfs_tree_unlock(free_space_root->node);
 	btrfs_free_tree_block(trans, btrfs_root_id(free_space_root),
 			      free_space_root->node, 0, 1);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a64a71d882dc..8048b536c682 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -707,7 +707,7 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
 		 * exists).
 		 */
 		btrfs_tree_lock(leaf);
-		btrfs_clean_tree_block(leaf);
+		btrfs_clear_buffer_dirty(leaf);
 		btrfs_tree_unlock(leaf);
 		btrfs_free_tree_block(trans, objectid, leaf, 0, 1);
 		free_extent_buffer(leaf);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 35856ea28e32..9e22698ea439 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1303,7 +1303,7 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
 	list_del(&quota_root->dirty_list);
 
 	btrfs_tree_lock(quota_root->node);
-	btrfs_clean_tree_block(quota_root->node);
+	btrfs_clear_buffer_dirty(quota_root->node);
 	btrfs_tree_unlock(quota_root->node);
 	btrfs_free_tree_block(trans, btrfs_root_id(quota_root),
 			      quota_root->node, 0, 1);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 73e621df32f7..15695f505f05 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2636,7 +2636,7 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
 				}
 
 				btrfs_tree_lock(next);
-				btrfs_clean_tree_block(next);
+				btrfs_clear_buffer_dirty(next);
 				btrfs_wait_tree_block_writeback(next);
 				btrfs_tree_unlock(next);
 
@@ -2705,7 +2705,7 @@ static noinline int walk_up_log_tree(struct btrfs_trans_handle *trans,
 				next = path->nodes[*level];
 
 				btrfs_tree_lock(next);
-				btrfs_clean_tree_block(next);
+				btrfs_clear_buffer_dirty(next);
 				btrfs_wait_tree_block_writeback(next);
 				btrfs_tree_unlock(next);
 
@@ -2786,7 +2786,7 @@ static int walk_log_tree(struct btrfs_trans_handle *trans,
 			next = path->nodes[orig_level];
 
 			btrfs_tree_lock(next);
-			btrfs_clean_tree_block(next);
+			btrfs_clear_buffer_dirty(next);
 			btrfs_wait_tree_block_writeback(next);
 			btrfs_tree_unlock(next);
 
-- 
2.26.3

[PATCH v2 8/9] btrfs: combine btrfs_clear_buffer_dirty and clear_extent_buffer_dirty

[Josef Bacik]

btrfs_clear_buffer_dirty just does the test_clear_bit() and then calls
clear_extent_buffer_dirty and does the dirty metadata accounting.
Combine this into clear_extent_buffer_dirty and make the result
btrfs_clear_buffer_dirty.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/disk-io.c   | 13 -------------
 fs/btrfs/extent_io.c | 11 ++++++++++-
 fs/btrfs/extent_io.h |  2 +-
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b8f1ac54d10c..8edbf73d8707 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -965,19 +965,6 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
 
 }
 
-void btrfs_clear_buffer_dirty(struct extent_buffer *buf)
-{
-	struct btrfs_fs_info *fs_info = buf->fs_info;
-	btrfs_assert_tree_write_locked(buf);
-
-	if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &buf->bflags)) {
-		percpu_counter_add_batch(&fs_info->dirty_metadata_bytes,
-					 -buf->len,
-					 fs_info->dirty_metadata_batch);
-		clear_extent_buffer_dirty(buf);
-	}
-}
-
 static void __setup_root(struct btrfs_root *root, struct btrfs_fs_info *fs_info,
 			 u64 objectid)
 {
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 7cd4065acc82..c242aabfa863 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4813,12 +4813,21 @@ static void clear_subpage_extent_buffer_dirty(const struct extent_buffer *eb)
 	WARN_ON(atomic_read(&eb->refs) == 0);
 }
 
-void clear_extent_buffer_dirty(const struct extent_buffer *eb)
+void btrfs_clear_buffer_dirty(struct extent_buffer *eb)
 {
+	struct btrfs_fs_info *fs_info = eb->fs_info;
 	int i;
 	int num_pages;
 	struct page *page;
 
+	btrfs_assert_tree_write_locked(eb);
+
+	if (!test_and_clear_bit(EXTENT_BUFFER_DIRTY, &eb->bflags))
+		return;
+
+	percpu_counter_add_batch(&fs_info->dirty_metadata_bytes, -eb->len,
+				 fs_info->dirty_metadata_batch);
+
 	if (eb->fs_info->nodesize < PAGE_SIZE)
 		return clear_subpage_extent_buffer_dirty(eb);
 
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index a024655d4237..41fc887d6cfe 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -261,7 +261,6 @@ void extent_buffer_bitmap_set(const struct extent_buffer *eb, unsigned long star
 void extent_buffer_bitmap_clear(const struct extent_buffer *eb,
 				unsigned long start, unsigned long pos,
 				unsigned long len);
-void clear_extent_buffer_dirty(const struct extent_buffer *eb);
 bool set_extent_buffer_dirty(struct extent_buffer *eb);
 void set_extent_buffer_uptodate(struct extent_buffer *eb);
 void clear_extent_buffer_uptodate(struct extent_buffer *eb);
@@ -273,6 +272,7 @@ void extent_clear_unlock_delalloc(struct btrfs_inode *inode, u64 start, u64 end,
 				  u32 bits_to_clear, unsigned long page_ops);
 int extent_invalidate_folio(struct extent_io_tree *tree,
 			    struct folio *folio, size_t offset);
+void btrfs_clear_buffer_dirty(struct extent_buffer *buf);
 
 int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array);
 
-- 
2.26.3

[PATCH v2 9/9] btrfs: remove btrfs_wait_tree_block_writeback

[Josef Bacik]

This is used in the tree-log code and is a holdover from previous
iterations of extent buffer writeback.  We can simply use
wait_on_extent_buffer_writeback here, and remove
btrfs_wait_tree_block_writeback completely.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/tree-log.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 15695f505f05..15f8130d812c 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -278,12 +278,6 @@ void btrfs_end_log_trans(struct btrfs_root *root)
 	}
 }
 
-static void btrfs_wait_tree_block_writeback(struct extent_buffer *buf)
-{
-	filemap_fdatawait_range(buf->pages[0]->mapping,
-			        buf->start, buf->start + buf->len - 1);
-}
-
 /*
  * the walk control struct is used to pass state down the chain when
  * processing the log tree.  The stage field tells us which part
@@ -2637,7 +2631,7 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
 
 				btrfs_tree_lock(next);
 				btrfs_clear_buffer_dirty(next);
-				btrfs_wait_tree_block_writeback(next);
+				wait_on_extent_buffer_writeback(next);
 				btrfs_tree_unlock(next);
 
 				if (trans) {
@@ -2706,7 +2700,7 @@ static noinline int walk_up_log_tree(struct btrfs_trans_handle *trans,
 
 				btrfs_tree_lock(next);
 				btrfs_clear_buffer_dirty(next);
-				btrfs_wait_tree_block_writeback(next);
+				wait_on_extent_buffer_writeback(next);
 				btrfs_tree_unlock(next);
 
 				if (trans) {
@@ -2787,7 +2781,7 @@ static int walk_log_tree(struct btrfs_trans_handle *trans,
 
 			btrfs_tree_lock(next);
 			btrfs_clear_buffer_dirty(next);
-			btrfs_wait_tree_block_writeback(next);
+			wait_on_extent_buffer_writeback(next);
 			btrfs_tree_unlock(next);
 
 			if (trans) {
-- 
2.26.3

Re: [PATCH v2 1/9] btrfs: do not call btrfs_clean_tree_block in update_ref_for_cow

[Qu Wenruo]

On 2023/1/20 05:53, Josef Bacik wrote:
> My series cleaning up the btrfs_clean_tree_block usage uncovered a
> pretty nasty problem, we weren't writing out metadata that was getting
> cow'ed away before the transaction commit was able to write out the
> block.
> 
> This was because I changed btrfs_clean_tree_block() to unconditionally
> clear the EXTENT_BUFFER_DIRTY flag, whereas before it was only doing it
> if the header generation == the transid of the currently running
> transaction.  This check exists purely for update_ref_for_cow's benefit,
> as we only really want to call this when we're sure we won't use the
> block again.  In the case of update_ref_for_cow() we're only truly
> wanting to avoid using the block if we're in the same transaction that
> dirtied it originally.  However we only get into update_ref_for_cow()
> with refs == 1 if we already wrote the block to disk via memory pressure
> or something, which means we won't have EXTENT_BUFFER_DIRTY set anyway.
> 
> Remove this usage in update_ref_for_cow(), as it doesn't make sense, and
> is actually harmful with my other cleanups.  We want to reserve the use
> of this function for when we free a block in the current transaction,
> like with delete's or balances.

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


To my understand and most part of update_ref_for_cow(), we really only 
need to inc/dec refs for a tree block when doing regular COW.

Thus this patch looks good to me.

But this also means, btrfs_clean_tree_block() should really only be 
called for tree blocks that got emptied (mostly by tree operations).

In that case, can we add an extra ASSERT() on btrfs_clean_tree_block() 
to make sure it's really empty?

Thanks,
Qu

> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   fs/btrfs/ctree.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 5476d90a76ce..ac33dbb08263 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -459,7 +459,6 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
>   			if (ret)
>   				return ret;
>   		}
> -		btrfs_clean_tree_block(buf);
>   		*last_ref = 1;
>   	}
>   	return 0;

Re: [PATCH v2 0/9] extent buffer dirty cleanups

[Wang Yugui]

Hi,

> v1->v2:
> - Added "btrfs: do not call btrfs_clean_tree_block in update_ref_for_cow", Qu
>   noticed some corruption with the original patchset, this turned out to be
>   because we were clearing the extent buffer dirty at cow time, which doesn't
>   make sense as we're not free'ing the current block in our current transaction.

With these 9 patches,  fstest btrfs/001 will fail.
Without these 9 patches,  fstest btrfs/001 passed.

kernel version:  linux kernel 6.1.8-rc2 with btrfs code of 6.2.0.
dmesg of 'fstest btrfs/001'
[  207.961928] run fstests btrfs/001 at 2023-01-24 14:33:30
[  208.153611] BTRFS info (device sdb1): using crc32c (crc32c-intel) checksum algorithm
[  208.161357] BTRFS info (device sdb1): using free space tree
[  208.168863] BTRFS info (device sdb1): enabling ssd optimizations
[  208.712457] BTRFS: device fsid a08f4da5-62be-4516-84d6-4a29b7d1b075 devid 1 transid 6 /dev/sdb2 scanned by systemd-udevd (3737)
[  208.745894] BTRFS info (device sdb2): using crc32c (crc32c-intel) checksum algorithm
[  208.753615] BTRFS info (device sdb2): using free space tree
[  208.760818] BTRFS info (device sdb2): enabling ssd optimizations
[  208.767016] BTRFS info (device sdb2): checking UUID tree
[  208.799067] ------------[ cut here ]------------
[  208.803671] WARNING: CPU: 5 PID: 3771 at fs/btrfs/extent-tree.c:3333 btrfs_free_tree_block+0x2a1/0x2b0 [btrfs]
[  208.813709] Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache netfs rpcrdma rdma_cm iw_cm ib_cm rfkill ib_core dm_multipath dm_mod intel_rapl_msr intel_rapl_common sb_edac snd_hda_codec_realtek x86_pkg_temp_thermal snd_hda_codec_generic intel_powerclamp snd_hda_codec_hdmi ledtrig_audio coretemp snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi kvm_intel snd_hda_codec btrfs kvm snd_hda_core snd_hwdep snd_seq irqbypass mei_wdt dcdbas iTCO_wdt crct10dif_pclmul snd_seq_device iTCO_vendor_support crc32_pclmul dell_smm_hwmon blake2b_generic ghash_clmulni_intel xor snd_pcm raid6_pq rapl snd_timer intel_cstate zstd_compress mei_me snd i2c_i801 intel_uncore pcspkr lpc_ich i2c_smbus mei soundcore nfsd auth_rpcgss nfs_acl lockd grace sunrpc xfs sd_mod t10_pi amdgpu iommu_v2 gpu_sched drm_buddy sr_mod cdrom sg radeon ahci libahci video bnx2x drm_ttm_helper mpt3sas ttm libata e1000e drm_display_helper crc32c_intel mdio cec raid_class scsi_transport_sas wmi i2c_dev
[  208.898933] CPU: 5 PID: 3771 Comm: btrfs Not tainted 6.1.8-0.5.el7.x86_64 #1
[  208.905948] Hardware name: Dell Inc. Precision T7610/0NK70N, BIOS A18 09/11/2019
[  208.913308] RIP: 0010:btrfs_free_tree_block+0x2a1/0x2b0 [btrfs]
[  208.919243] Code: 01 00 00 00 4c 89 e6 e8 3d a2 ff ff 4c 89 e7 e8 25 b4 0a 00 e9 7d fe ff ff 49 8b 7d 20 48 89 ee e8 d4 cc 0b 00 e9 6c fe ff ff <0f> 0b e9 11 ff ff ff e8 83 2c 51 c1 0f 1f 00 0f 1f 44 00 00 8b 06
[  208.937913] RSP: 0018:ffffb43e63e5f7d8 EFLAGS: 00010202
[  208.943117] RAX: 0000000000000213 RBX: ffff9b9207576000 RCX: ffff9b92a16cd8d8
[  208.950220] RDX: 0000000000000000 RSI: 0000000000540000 RDI: ffff9b9207576088
[  208.957321] RBP: ffff9b9208e7d500 R08: 0000000000000000 R09: 0000000000480000
[  208.964425] R10: 0000000000000000 R11: ffff9b9207576a40 R12: ffff9b92a16cd800
[  208.971525] R13: ffff9b923abaa548 R14: 0000000000000001 R15: ffff9b9208e7cd00
[  208.978627] FS:  00007f390b354a00(0000) GS:ffff9bb0ef940000(0000) knlGS:0000000000000000
[  208.986680] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  208.992398] CR2: 00000000015b59e8 CR3: 0000000108efe006 CR4: 00000000001706e0
[  208.999501] Call Trace:
[  209.001940]  <TASK>
[  209.004037]  __btrfs_cow_block+0x48b/0x510 [btrfs]
[  209.008853]  btrfs_cow_block+0xf2/0x190 [btrfs]
[  209.013426]  btrfs_search_slot+0x6d6/0xbc0 [btrfs]
[  209.018238]  btrfs_insert_empty_items+0x31/0x70 [btrfs]
[  209.023482]  btrfs_insert_delayed_items+0x22f/0x430 [btrfs]
[  209.029097]  __btrfs_run_delayed_items+0x9c/0x190 [btrfs]
[  209.034539]  ? create_pending_snapshots+0x92/0xc0 [btrfs]
[  209.039958]  btrfs_commit_transaction+0x28d/0xba0 [btrfs]
[  209.045379]  ? start_transaction+0xd0/0x5e0 [btrfs]
[  209.050279]  btrfs_mksubvol+0x364/0x570 [btrfs]
[  209.054848]  btrfs_mksnapshot+0x7c/0xb0 [btrfs]
[  209.059442]  __btrfs_ioctl_snap_create+0x17b/0x190 [btrfs]
[  209.064957]  btrfs_ioctl_snap_create_v2+0x11c/0x140 [btrfs]
[  209.070557]  btrfs_ioctl+0x91c/0x26b0 [btrfs]
[  209.074952]  ? __mod_memcg_lruvec_state+0x66/0xd0
[  209.079645]  ? __mod_lruvec_page_state+0x85/0x130
[  209.084333]  ? page_add_new_anon_rmap+0x8c/0x1f0
[  209.088937]  ? folio_add_lru+0x7d/0xe0
[  209.092677]  ? do_anonymous_page+0x283/0x320
[  209.096938]  ? __x64_sys_ioctl+0x89/0xc0
[  209.100853]  __x64_sys_ioctl+0x89/0xc0
[  209.104594]  do_syscall_64+0x58/0x80
[  209.108164]  ? handle_mm_fault+0xfb/0x2f0
[  209.112167]  ? do_user_addr_fault+0x1eb/0x6a0
[  209.116512]  ? exc_page_fault+0x64/0x140
[  209.120426]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[  209.125463] RIP: 0033:0x7f39090397cb
[  209.129030] Code: 73 01 c3 48 8b 0d bd 66 38 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8d 66 38 00 f7 d8 64 89 01 48
[  209.147702] RSP: 002b:00007ffd86214868 EFLAGS: 00000202 ORIG_RAX: 0000000000000010
[  209.155238] RAX: ffffffffffffffda RBX: 00000000015a4920 RCX: 00007f39090397cb
[  209.162342] RDX: 00007ffd862148b0 RSI: 0000000050009417 RDI: 0000000000000003
[  209.169445] RBP: 0000000000000004 R08: 0000000000000000 R09: 00007f3909180d40
[  209.176549] R10: 0000000000000010 R11: 0000000000000202 R12: 00000000015a4940
[  209.183653] R13: 00000000015a4940 R14: 00007ffd862148b0 R15: 0000000000000000
[  209.190758]  </TASK>
[  209.192938] ---[ end trace 0000000000000000 ]---
[  209.197624] ------------[ cut here ]------------
[  209.202230] WARNING: CPU: 25 PID: 3771 at fs/btrfs/extent-tree.c:3333 btrfs_free_tree_block+0x2a1/0x2b0 [btrfs]
[  209.212311] Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache netfs rpcrdma rdma_cm iw_cm ib_cm rfkill ib_core dm_multipath dm_mod intel_rapl_msr intel_rapl_common sb_edac snd_hda_codec_realtek x86_pkg_temp_thermal snd_hda_codec_generic intel_powerclamp snd_hda_codec_hdmi ledtrig_audio coretemp snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi kvm_intel snd_hda_codec btrfs kvm snd_hda_core snd_hwdep snd_seq irqbypass mei_wdt dcdbas iTCO_wdt crct10dif_pclmul snd_seq_device iTCO_vendor_support crc32_pclmul dell_smm_hwmon blake2b_generic ghash_clmulni_intel xor snd_pcm raid6_pq rapl snd_timer intel_cstate zstd_compress mei_me snd i2c_i801 intel_uncore pcspkr lpc_ich i2c_smbus mei soundcore nfsd auth_rpcgss nfs_acl lockd grace sunrpc xfs sd_mod t10_pi amdgpu iommu_v2 gpu_sched drm_buddy sr_mod cdrom sg radeon ahci libahci video bnx2x drm_ttm_helper mpt3sas ttm libata e1000e drm_display_helper crc32c_intel mdio cec raid_class scsi_transport_sas wmi i2c_dev
[  209.297508] CPU: 25 PID: 3771 Comm: btrfs Tainted: G        W          6.1.8-0.5.el7.x86_64 #1
[  209.306076] Hardware name: Dell Inc. Precision T7610/0NK70N, BIOS A18 09/11/2019
[  209.313433] RIP: 0010:btrfs_free_tree_block+0x2a1/0x2b0 [btrfs]
[  209.319367] Code: 01 00 00 00 4c 89 e6 e8 3d a2 ff ff 4c 89 e7 e8 25 b4 0a 00 e9 7d fe ff ff 49 8b 7d 20 48 89 ee e8 d4 cc 0b 00 e9 6c fe ff ff <0f> 0b e9 11 ff ff ff e8 83 2c 51 c1 0f 1f 00 0f 1f 44 00 00 8b 06
[  209.338034] RSP: 0018:ffffb43e63e5f818 EFLAGS: 00010202
[  209.343233] RAX: 0000000000000213 RBX: ffff9b9207576000 RCX: ffff9b92a16cd8d8
[  209.350332] RDX: 0000000000000000 RSI: 0000000000560000 RDI: ffff9b9207576088
[  209.357431] RBP: ffff9b9208e7cd00 R08: 0000000000000001 R09: 0000000000000000
[  209.364529] R10: 00000000c4ed3201 R11: 0000000000000001 R12: ffff9b92a16cd800
[  209.371628] R13: ffff9b923abaa548 R14: 0000000000000001 R15: ffff9b92a35fd100
[  209.378724] FS:  00007f390b354a00(0000) GS:ffff9bb0efbc0000(0000) knlGS:0000000000000000
[  209.386773] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  209.392489] CR2: 000056301d9aaae0 CR3: 0000000108efe006 CR4: 00000000001706e0
[  209.399588] Call Trace:
[  209.402024]  <TASK>
[  209.404116]  __btrfs_cow_block+0x48b/0x510 [btrfs]
[  209.408926]  btrfs_cow_block+0xf2/0x190 [btrfs]
[  209.413476]  btrfs_search_slot+0x6d6/0xbc0 [btrfs]
[  209.418285]  ? btrfs_block_rsv_release+0x194/0x2e0 [btrfs]
[  209.423806]  btrfs_lookup_inode+0x3a/0xc0 [btrfs]
[  209.428535]  __btrfs_update_delayed_inode+0x73/0x260 [btrfs]
[  209.434225]  __btrfs_run_delayed_items+0x10b/0x190 [btrfs]
[  209.439749]  ? create_pending_snapshots+0x92/0xc0 [btrfs]
[  209.445168]  btrfs_commit_transaction+0x28d/0xba0 [btrfs]
[  209.450587]  ? start_transaction+0xd0/0x5e0 [btrfs]
[  209.455486]  btrfs_mksubvol+0x364/0x570 [btrfs]
[  209.460058]  btrfs_mksnapshot+0x7c/0xb0 [btrfs]
[  209.464622]  __btrfs_ioctl_snap_create+0x17b/0x190 [btrfs]
[  209.470145]  btrfs_ioctl_snap_create_v2+0x11c/0x140 [btrfs]
[  209.475754]  btrfs_ioctl+0x91c/0x26b0 [btrfs]
[  209.480157]  ? __mod_memcg_lruvec_state+0x66/0xd0
[  209.484839]  ? __mod_lruvec_page_state+0x85/0x130
[  209.489521]  ? page_add_new_anon_rmap+0x8c/0x1f0
[  209.494121]  ? folio_add_lru+0x7d/0xe0
[  209.497854]  ? do_anonymous_page+0x283/0x320
[  209.502105]  ? __x64_sys_ioctl+0x89/0xc0
[  209.506013]  __x64_sys_ioctl+0x89/0xc0
[  209.509747]  do_syscall_64+0x58/0x80
[  209.513309]  ? handle_mm_fault+0xfb/0x2f0
[  209.517301]  ? do_user_addr_fault+0x1eb/0x6a0
[  209.521641]  ? exc_page_fault+0x64/0x140
[  209.525549]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[  209.530577] RIP: 0033:0x7f39090397cb
[  209.534137] Code: 73 01 c3 48 8b 0d bd 66 38 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8d 66 38 00 f7 d8 64 89 01 48
[  209.552806] RSP: 002b:00007ffd86214868 EFLAGS: 00000202 ORIG_RAX: 0000000000000010
[  209.560337] RAX: ffffffffffffffda RBX: 00000000015a4920 RCX: 00007f39090397cb
[  209.567436] RDX: 00007ffd862148b0 RSI: 0000000050009417 RDI: 0000000000000003
[  209.574537] RBP: 0000000000000004 R08: 0000000000000000 R09: 00007f3909180d40
[  209.581634] R10: 0000000000000010 R11: 0000000000000202 R12: 00000000015a4940
[  209.588731] R13: 00000000015a4940 R14: 00007ffd862148b0 R15: 0000000000000000
[  209.595830]  </TASK>
[  209.598009] ---[ end trace 0000000000000000 ]---
[  209.627004] BTRFS info (device sdb2): setting incompat feature flag for DEFAULT_SUBVOL (0x2)
[  209.646478] BTRFS info (device sdb2): using crc32c (crc32c-intel) checksum algorithm
[  209.654195] BTRFS info (device sdb2): using free space tree
[  209.661661] BTRFS info (device sdb2): enabling ssd optimizations
[  209.676674] BTRFS info (device sdb2): using crc32c (crc32c-intel) checksum algorithm
[  209.684389] BTRFS info (device sdb2): using free space tree
[  209.691756] BTRFS info (device sdb2): enabling ssd optimizations
[  209.709825] BTRFS info (device sdb2): using crc32c (crc32c-intel) checksum algorithm
[  209.717539] BTRFS info (device sdb2): using free space tree
[  209.724810] BTRFS info (device sdb2): enabling ssd optimizations
[  209.748422] BTRFS info (device sdb2): using crc32c (crc32c-intel) checksum algorithm
[  209.756155] BTRFS info (device sdb2): using free space tree
[  209.763554] BTRFS info (device sdb2): enabling ssd optimizations
[  209.814801] BTRFS info (device sdb2): using crc32c (crc32c-intel) checksum algorithm
[  209.822530] BTRFS info (device sdb2): using free space tree
[  209.830248] BTRFS info (device sdb2): enabling ssd optimizations



Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2023/01/24

> 
> --- Original email ---
> Hello,
> 
> While sync'ing ctree.c to btrfs-progs I noticed we have some oddities when it
> comes to how we deal with the extent buffer being dirty.  We have
> btrfs_clean_tree_block, which is sort of meant to be run against extent buffers
> we've modified in this transaction.  However we have some other places where
> we've open coded the same work without the generation check.  This makes it kind
> of confusing, and is inconsistent with how we deal with the
> fs_info->dirty_metadata_bytes.
> 
> So clean this stuff up so we have one helper we use for setting the extent
> buffer dirty (btrfs_mark_buffer_dirty) and one for clearing dirty
> (btrfs_clear_buffer_dirty).  This makes everything more consistent and clean
> across the board.  I've additionally cleaned up a random writeback thing we had
> in tree-log that I noticed while doing these cleanups.  Thanks,
> 
> Josef
> 
> Josef Bacik (9):
>   btrfs: do not call btrfs_clean_tree_block in update_ref_for_cow
>   btrfs: always lock the block before calling btrfs_clean_tree_block
>   btrfs: do not check header generation in btrfs_clean_tree_block
>   btrfs: do not set the header generation before btrfs_clean_tree_block
>   btrfs: replace clearing extent buffer dirty bit with btrfs_clean_block
>   btrfs: do not increment dirty_metadata_bytes in set_btree_ioerr
>   btrfs: rename btrfs_clean_tree_block => btrfs_clear_buffer_dirty
>   btrfs: combine btrfs_clear_buffer_dirty and clear_extent_buffer_dirty
>   btrfs: remove btrfs_wait_tree_block_writeback
> 
>  fs/btrfs/ctree.c           | 15 +++++++-------
>  fs/btrfs/disk-io.c         | 25 +++++-------------------
>  fs/btrfs/disk-io.h         |  2 +-
>  fs/btrfs/extent-tree.c     | 12 ++++--------
>  fs/btrfs/extent_io.c       | 18 +++++++++--------
>  fs/btrfs/extent_io.h       |  2 +-
>  fs/btrfs/free-space-tree.c |  2 +-
>  fs/btrfs/ioctl.c           |  2 +-
>  fs/btrfs/qgroup.c          |  2 +-
>  fs/btrfs/tree-log.c        | 40 ++++++++++++++------------------------
>  10 files changed, 46 insertions(+), 74 deletions(-)
> 
> -- 
> 2.26.3