[PATCH 00/10] btrfs: improve error reporting for log tree replay

[PATCH 00/10] btrfs: improve error reporting for log tree replay

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Most errors that happen during log replay or destroying a log tree are hard
to figure out where they come from, since typically deep in the call chain
of log tree walking we return errors and propagate them up to the caller of
the main log tree walk function, which then aborts the transaction or turns
the filesystem into error state (btrfs_handle_fs_error()). This means any
stack trace and message provided by a transaction abort or turning fs into
error state, doesn't provide information where exactly in the deep call
chain the error comes from.

These changes mostly make transacton aborts and btrfs_handle_fs_error()
calls where errors happen, so that we get much more useful information
which sometimes is enough to understand issues. The rest are just some
cleanups.

Filipe Manana (10):
  btrfs: error on missing block group when unaccounting log tree extent buffers
  btrfs: abort transaction on specific error places when walking log tree
  btrfs: abort transaction in the process_one_buffer() log tree walk callback
  btrfs: use local variable for the transaction handle in replay_one_buffer()
  btrfs: return real error from read_alloc_one_name() in drop_one_dir_item()
  btrfs: abort transaction where errors happen during log tree replay
  btrfs: exit early when replaying hole file extent item from a log tree
  btrfs: process inline extent earlier in replay_one_extent()
  btrfs: use local key variable to pass arguments in replay_one_extent()
  btrfs: collapse unaccount_log_buffer() into clean_log_buffer()

 fs/btrfs/tree-log.c | 659 +++++++++++++++++++++++++++-----------------
 1 file changed, 401 insertions(+), 258 deletions(-)

-- 
2.47.2

[PATCH 01/10] btrfs: error on missing block group when unaccounting log tree extent buffers

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Currently we only log an error message if we can't find the block group
for a log tree extent buffer when unaccounting it (while freeing a log
tree). A missing block group means something is seriously wrong and we
end up leaking space from the metadata space info. So return -ENOENT in
case we don't find the block group.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 8ad6005257b8..45f2e13f5018 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2593,14 +2593,14 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 /*
  * Correctly adjust the reserved bytes occupied by a log tree extent buffer
  */
-static void unaccount_log_buffer(struct btrfs_fs_info *fs_info, u64 start)
+static int unaccount_log_buffer(struct btrfs_fs_info *fs_info, u64 start)
 {
 	struct btrfs_block_group *cache;
 
 	cache = btrfs_lookup_block_group(fs_info, start);
 	if (!cache) {
 		btrfs_err(fs_info, "unable to find block group for %llu", start);
-		return;
+		return -ENOENT;
 	}
 
 	spin_lock(&cache->space_info->lock);
@@ -2611,27 +2611,22 @@ static void unaccount_log_buffer(struct btrfs_fs_info *fs_info, u64 start)
 	spin_unlock(&cache->space_info->lock);
 
 	btrfs_put_block_group(cache);
+
+	return 0;
 }
 
 static int clean_log_buffer(struct btrfs_trans_handle *trans,
 			    struct extent_buffer *eb)
 {
-	int ret;
-
 	btrfs_tree_lock(eb);
 	btrfs_clear_buffer_dirty(trans, eb);
 	wait_on_extent_buffer_writeback(eb);
 	btrfs_tree_unlock(eb);
 
-	if (trans) {
-		ret = btrfs_pin_reserved_extent(trans, eb);
-		if (ret)
-			return ret;
-	} else {
-		unaccount_log_buffer(eb->fs_info, eb->start);
-	}
+	if (trans)
+		return btrfs_pin_reserved_extent(trans, eb);
 
-	return 0;
+	return unaccount_log_buffer(eb->fs_info, eb->start);
 }
 
 static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
-- 
2.47.2

[PATCH 02/10] btrfs: abort transaction on specific error places when walking log tree

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

We do several things while walking a log tree (for replaying and for
freeing a log tree) like reading extent buffers and cleaning them up,
but we don't immediately abort the transaction, or turn the fs into an
error state, when one of these things fails. Instead we the transaction
abort or turn the fs into error state in the caller of the entry point
function that walks a log tree - walk_log_tree() - which means we don't
get to know exactly where an error came from.

Improve on this by doing a transaction abort / turn fs into error state
after each such failure so that when it happens we have a better
understanding where the failure comes from. This deliberately leaves
the transaction abort / turn fs into error state in the callers of
walk_log_tree() as to ensure we don't get into an inconsitent state in
case we forget to do it deeper in call chain. It also deliberately does
not do it after errors from the calls to the callback defined in
struct walk_control::process_func(), as we will do it later on another
patch.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 45f2e13f5018..b5b1f38c03a6 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2618,15 +2618,24 @@ static int unaccount_log_buffer(struct btrfs_fs_info *fs_info, u64 start)
 static int clean_log_buffer(struct btrfs_trans_handle *trans,
 			    struct extent_buffer *eb)
 {
+	int ret;
+
 	btrfs_tree_lock(eb);
 	btrfs_clear_buffer_dirty(trans, eb);
 	wait_on_extent_buffer_writeback(eb);
 	btrfs_tree_unlock(eb);
 
-	if (trans)
-		return btrfs_pin_reserved_extent(trans, eb);
+	if (trans) {
+		ret = btrfs_pin_reserved_extent(trans, eb);
+		if (ret)
+			btrfs_abort_transaction(trans, ret);
+		return ret;
+	}
 
-	return unaccount_log_buffer(eb->fs_info, eb->start);
+	ret = unaccount_log_buffer(eb->fs_info, eb->start);
+	if (ret)
+		btrfs_handle_fs_error(eb->fs_info, ret, NULL);
+	return ret;
 }
 
 static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
@@ -2662,8 +2671,14 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
 		next = btrfs_find_create_tree_block(fs_info, bytenr,
 						    btrfs_header_owner(cur),
 						    *level - 1);
-		if (IS_ERR(next))
-			return PTR_ERR(next);
+		if (IS_ERR(next)) {
+			ret = PTR_ERR(next);
+			if (trans)
+				btrfs_abort_transaction(trans, ret);
+			else
+				btrfs_handle_fs_error(fs_info, ret, NULL);
+			return ret;
+		}
 
 		if (*level == 1) {
 			ret = wc->process_func(root, next, wc, ptr_gen,
@@ -2678,6 +2693,10 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
 				ret = btrfs_read_extent_buffer(next, &check);
 				if (ret) {
 					free_extent_buffer(next);
+					if (trans)
+						btrfs_abort_transaction(trans, ret);
+					else
+						btrfs_handle_fs_error(fs_info, ret, NULL);
 					return ret;
 				}
 
@@ -2693,6 +2712,10 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
 		ret = btrfs_read_extent_buffer(next, &check);
 		if (ret) {
 			free_extent_buffer(next);
+			if (trans)
+				btrfs_abort_transaction(trans, ret);
+			else
+				btrfs_handle_fs_error(fs_info, ret, NULL);
 			return ret;
 		}
 
-- 
2.47.2

[PATCH 03/10] btrfs: abort transaction in the process_one_buffer() log tree walk callback

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

In the process_one_buffer() log tree walk callback we return errors to the
log tree walk caller and then the caller aborts the transaction, if we
have one, or turns the fs into error state if we don't have one. While
this reduces code it makes it harder to figure out where exactly an error
came from. So add the transaction aborts after every failure inside the
process_one_buffer() callback, so that it helps figuring out why failures
happen.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index b5b1f38c03a6..5e0c4c0595a7 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -348,6 +348,7 @@ static int process_one_buffer(struct btrfs_root *log,
 			      struct extent_buffer *eb,
 			      struct walk_control *wc, u64 gen, int level)
 {
+	struct btrfs_trans_handle *trans = wc->trans;
 	struct btrfs_fs_info *fs_info = log->fs_info;
 	int ret = 0;
 
@@ -362,18 +363,35 @@ static int process_one_buffer(struct btrfs_root *log,
 		};
 
 		ret = btrfs_read_extent_buffer(eb, &check);
-		if (ret)
+		if (ret) {
+			if (trans)
+				btrfs_abort_transaction(trans, ret);
+			else
+				btrfs_handle_fs_error(fs_info, ret, NULL);
 			return ret;
+		}
 	}
 
 	if (wc->pin) {
-		ret = btrfs_pin_extent_for_log_replay(wc->trans, eb);
-		if (ret)
+		ret = btrfs_pin_extent_for_log_replay(trans, eb);
+		if (ret) {
+			if (trans)
+				btrfs_abort_transaction(trans, ret);
+			else
+				btrfs_handle_fs_error(fs_info, ret, NULL);
 			return ret;
+		}
 
 		if (btrfs_buffer_uptodate(eb, gen, 0) &&
-		    btrfs_header_level(eb) == 0)
+		    btrfs_header_level(eb) == 0) {
 			ret = btrfs_exclude_logged_extents(eb);
+			if (ret) {
+				if (trans)
+					btrfs_abort_transaction(trans, ret);
+				else
+					btrfs_handle_fs_error(fs_info, ret, NULL);
+			}
+		}
 	}
 	return ret;
 }
-- 
2.47.2

[PATCH 04/10] btrfs: use local variable for the transaction handle in replay_one_buffer()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Instead of keep dereferencing the walk_control structure to extract the
transaction handle whenever is needed, do it once by storing it in a local
variable and then use the variable everywhere. This reduces code verbosity
and eliminates the need for some split lines.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 5e0c4c0595a7..0fff6716a7ed 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2464,6 +2464,7 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 	};
 	struct btrfs_path *path;
 	struct btrfs_root *root = wc->replay_dest;
+	struct btrfs_trans_handle *trans = wc->trans;
 	struct btrfs_key key;
 	int i;
 	int ret;
@@ -2507,19 +2508,17 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 			} else {
 				wc->ignore_cur_inode = false;
 			}
-			ret = replay_xattr_deletes(wc->trans, root, log,
-						   path, key.objectid);
+			ret = replay_xattr_deletes(trans, root, log, path, key.objectid);
 			if (ret)
 				break;
 			mode = btrfs_inode_mode(eb, inode_item);
 			if (S_ISDIR(mode)) {
-				ret = replay_dir_deletes(wc->trans, root, log, path,
+				ret = replay_dir_deletes(trans, root, log, path,
 							 key.objectid, false);
 				if (ret)
 					break;
 			}
-			ret = overwrite_item(wc->trans, root, path,
-					     eb, i, &key);
+			ret = overwrite_item(trans, root, path, eb, i, &key);
 			if (ret)
 				break;
 
@@ -2546,21 +2545,19 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 				drop_args.start = from;
 				drop_args.end = (u64)-1;
 				drop_args.drop_cache = true;
-				ret = btrfs_drop_extents(wc->trans, root, inode,
-							 &drop_args);
+				ret = btrfs_drop_extents(trans, root, inode,  &drop_args);
 				if (!ret) {
 					inode_sub_bytes(&inode->vfs_inode,
 							drop_args.bytes_found);
 					/* Update the inode's nbytes. */
-					ret = btrfs_update_inode(wc->trans, inode);
+					ret = btrfs_update_inode(trans, inode);
 				}
 				iput(&inode->vfs_inode);
 				if (ret)
 					break;
 			}
 
-			ret = link_to_fixup_dir(wc->trans, root,
-						path, key.objectid);
+			ret = link_to_fixup_dir(trans, root, path, key.objectid);
 			if (ret)
 				break;
 		}
@@ -2570,8 +2567,7 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 
 		if (key.type == BTRFS_DIR_INDEX_KEY &&
 		    wc->stage == LOG_WALK_REPLAY_DIR_INDEX) {
-			ret = replay_one_dir_item(wc->trans, root, path,
-						  eb, i, &key);
+			ret = replay_one_dir_item(trans, root, path, eb, i, &key);
 			if (ret)
 				break;
 		}
@@ -2581,19 +2577,16 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 
 		/* these keys are simply copied */
 		if (key.type == BTRFS_XATTR_ITEM_KEY) {
-			ret = overwrite_item(wc->trans, root, path,
-					     eb, i, &key);
+			ret = overwrite_item(trans, root, path, eb, i, &key);
 			if (ret)
 				break;
 		} else if (key.type == BTRFS_INODE_REF_KEY ||
 			   key.type == BTRFS_INODE_EXTREF_KEY) {
-			ret = add_inode_ref(wc->trans, root, log, path,
-					    eb, i, &key);
+			ret = add_inode_ref(trans, root, log, path, eb, i, &key);
 			if (ret)
 				break;
 		} else if (key.type == BTRFS_EXTENT_DATA_KEY) {
-			ret = replay_one_extent(wc->trans, root, path,
-						eb, i, &key);
+			ret = replay_one_extent(trans, root, path, eb, i, &key);
 			if (ret)
 				break;
 		}
-- 
2.47.2

[PATCH 05/10] btrfs: return real error from read_alloc_one_name() in drop_one_dir_item()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

If read_alloc_one_name() we explicitly return -ENOMEM and currently that
is fine since it's the only error read_alloc_one_name() can return for
now. However this is fragile and not future proof, so return instead what
read_alloc_one_name() returned.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 0fff6716a7ed..1c6210786a87 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -950,7 +950,7 @@ static noinline int drop_one_dir_item(struct btrfs_trans_handle *trans,
 	btrfs_dir_item_key_to_cpu(leaf, di, &location);
 	ret = read_alloc_one_name(leaf, di + 1, btrfs_dir_name_len(leaf, di), &name);
 	if (ret)
-		return -ENOMEM;
+		return ret;
 
 	btrfs_release_path(path);
 
-- 
2.47.2

[PATCH 06/10] btrfs: abort transaction where errors happen during log tree replay

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

In the replay_one_buffer() log tree walk callback we return errors to the
log tree walk caller and then the caller aborts the transaction, if we
have one, or turns the fs into error state if we don't have one. While
this reduces code it makes it harder to figure out where exactly an error
came from. So add the transaction aborts after every failure inside the
replay_one_buffer() callback and the functions it calls, making it as
fine grained as possible, so that it helps figuring out why failures
happen.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 1c6210786a87..17af4ff51479 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -440,8 +440,10 @@ static int overwrite_item(struct btrfs_trans_handle *trans,
 
 	/* Look for the key in the destination tree. */
 	ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
-	if (ret < 0)
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 		return ret;
+	}
 
 	dst_eb = path->nodes[0];
 	dst_slot = path->slots[0];
@@ -460,6 +462,7 @@ static int overwrite_item(struct btrfs_trans_handle *trans,
 		src_copy = kmalloc(item_size, GFP_NOFS);
 		if (!src_copy) {
 			btrfs_release_path(path);
+			btrfs_abort_transaction(trans, -ENOMEM);
 			return -ENOMEM;
 		}
 
@@ -544,6 +547,7 @@ static int overwrite_item(struct btrfs_trans_handle *trans,
 		else if (found_size < item_size)
 			btrfs_extend_item(trans, path, item_size - found_size);
 	} else if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		return ret;
 	}
 	dst_ptr = btrfs_item_ptr_offset(dst_eb, dst_slot);
@@ -674,6 +678,7 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 		extent_end = ALIGN(start + size,
 				   fs_info->sectorsize);
 	} else {
+		btrfs_abort_transaction(trans, -EUCLEAN);
 		btrfs_err(fs_info,
 		  "unexpected extent type=%d root=%llu inode=%llu offset=%llu",
 			  found_type, btrfs_root_id(root), key->objectid, key->offset);
@@ -681,8 +686,11 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 	}
 
 	inode = btrfs_iget_logging(key->objectid, root);
-	if (IS_ERR(inode))
-		return PTR_ERR(inode);
+	if (IS_ERR(inode)) {
+		ret = PTR_ERR(inode);
+		btrfs_abort_transaction(trans, ret);
+		return ret;
+	}
 
 	/*
 	 * first check to see if we already have this extent in the
@@ -717,8 +725,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 	drop_args.end = extent_end;
 	drop_args.drop_cache = true;
 	ret = btrfs_drop_extents(trans, root, inode, &drop_args);
-	if (ret)
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 
 	if (found_type == BTRFS_FILE_EXTENT_REG ||
 	    found_type == BTRFS_FILE_EXTENT_PREALLOC) {
@@ -732,8 +742,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 
 		ret = btrfs_insert_empty_item(trans, root, path, key,
 					      sizeof(*item));
-		if (ret)
+		if (ret) {
+			btrfs_abort_transaction(trans, ret);
 			goto out;
+		}
 		dest_offset = btrfs_item_ptr_offset(path->nodes[0],
 						    path->slots[0]);
 		copy_extent_buffer(path->nodes[0], eb, dest_offset,
@@ -755,8 +767,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 		ret = btrfs_qgroup_trace_extent(trans,
 				btrfs_file_extent_disk_bytenr(eb, item),
 				btrfs_file_extent_disk_num_bytes(eb, item));
-		if (ret < 0)
+		if (ret < 0) {
+			btrfs_abort_transaction(trans, ret);
 			goto out;
+		}
 
 		if (ins.objectid > 0) {
 			u64 csum_start;
@@ -770,6 +784,7 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 			ret = btrfs_lookup_data_extent(fs_info, ins.objectid,
 						ins.offset);
 			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
 			} else if (ret == 0) {
 				struct btrfs_ref ref = {
@@ -782,8 +797,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 				btrfs_init_data_ref(&ref, key->objectid, offset,
 						    0, false);
 				ret = btrfs_inc_extent_ref(trans, &ref);
-				if (ret)
+				if (ret) {
+					btrfs_abort_transaction(trans, ret);
 					goto out;
+				}
 			} else {
 				/*
 				 * insert the extent pointer in the extent
@@ -792,8 +809,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 				ret = btrfs_alloc_logged_file_extent(trans,
 						btrfs_root_id(root),
 						key->objectid, offset, &ins);
-				if (ret)
+				if (ret) {
+					btrfs_abort_transaction(trans, ret);
 					goto out;
+				}
 			}
 			btrfs_release_path(path);
 
@@ -810,8 +829,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 			ret = btrfs_lookup_csums_list(root->log_root,
 						csum_start, csum_end - 1,
 						&ordered_sums, false);
-			if (ret < 0)
+			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
+			}
 			ret = 0;
 			/*
 			 * Now delete all existing cums in the csum root that
@@ -871,14 +892,20 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 							list);
 				csum_root = btrfs_csum_root(fs_info,
 							    sums->logical);
-				if (!ret)
+				if (!ret) {
 					ret = btrfs_del_csums(trans, csum_root,
 							      sums->logical,
 							      sums->len);
-				if (!ret)
+					if (ret)
+						btrfs_abort_transaction(trans, ret);
+				}
+				if (!ret) {
 					ret = btrfs_csum_file_blocks(trans,
 								     csum_root,
 								     sums);
+					if (ret)
+						btrfs_abort_transaction(trans, ret);
+				}
 				list_del(&sums->list);
 				kfree(sums);
 			}
@@ -895,12 +922,16 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 	}
 
 	ret = btrfs_inode_set_file_extent_range(inode, start, extent_end - start);
-	if (ret)
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 
 update_inode:
 	btrfs_update_inode_bytes(inode, nbytes, drop_args.bytes_found);
 	ret = btrfs_update_inode(trans, inode);
+	if (ret)
+		btrfs_abort_transaction(trans, ret);
 out:
 	iput(&inode->vfs_inode);
 	return ret;
@@ -914,15 +945,20 @@ static int unlink_inode_for_log_replay(struct btrfs_trans_handle *trans,
 	int ret;
 
 	ret = btrfs_unlink_inode(trans, dir, inode, name);
-	if (ret)
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		return ret;
+	}
 	/*
 	 * Whenever we need to check if a name exists or not, we check the
 	 * fs/subvolume tree. So after an unlink we must run delayed items, so
 	 * that future checks for a name during log replay see that the name
 	 * does not exists anymore.
 	 */
-	return btrfs_run_delayed_items(trans);
+	ret = btrfs_run_delayed_items(trans);
+	if (ret)
+		btrfs_abort_transaction(trans, ret);
+	return ret;
 }
 
 /*
@@ -949,14 +985,17 @@ static noinline int drop_one_dir_item(struct btrfs_trans_handle *trans,
 
 	btrfs_dir_item_key_to_cpu(leaf, di, &location);
 	ret = read_alloc_one_name(leaf, di + 1, btrfs_dir_name_len(leaf, di), &name);
-	if (ret)
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		return ret;
+	}
 
 	btrfs_release_path(path);
 
 	inode = btrfs_iget_logging(location.objectid, root);
 	if (IS_ERR(inode)) {
 		ret = PTR_ERR(inode);
+		btrfs_abort_transaction(trans, ret);
 		inode = NULL;
 		goto out;
 	}
@@ -1087,14 +1126,18 @@ static int unlink_refs_not_in_log(struct btrfs_trans_handle *trans,
 		ret = read_alloc_one_name(leaf, (victim_ref + 1),
 					  btrfs_inode_ref_name_len(leaf, victim_ref),
 					  &victim_name);
-		if (ret)
+		if (ret) {
+			btrfs_abort_transaction(trans, ret);
 			return ret;
+		}
 
 		ret = backref_in_log(log_root, search_key, parent_objectid, &victim_name);
 		if (ret) {
 			kfree(victim_name.name);
-			if (ret < 0)
+			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
 				return ret;
+			}
 			ptr = (unsigned long)(victim_ref + 1) + victim_name.len;
 			continue;
 		}
@@ -1140,8 +1183,10 @@ static int unlink_extrefs_not_in_log(struct btrfs_trans_handle *trans,
 
 		ret = read_alloc_one_name(leaf, &extref->name, victim_name.len,
 					  &victim_name);
-		if (ret)
+		if (ret) {
+			btrfs_abort_transaction(trans, ret);
 			return ret;
+		}
 
 		search_key->objectid = inode_objectid;
 		search_key->type = BTRFS_INODE_EXTREF_KEY;
@@ -1151,8 +1196,10 @@ static int unlink_extrefs_not_in_log(struct btrfs_trans_handle *trans,
 		ret = backref_in_log(log_root, search_key, parent_objectid, &victim_name);
 		if (ret) {
 			kfree(victim_name.name);
-			if (ret < 0)
+			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
 				return ret;
+			}
 next:
 			cur_offset += victim_name.len + sizeof(*extref);
 			continue;
@@ -1161,7 +1208,9 @@ static int unlink_extrefs_not_in_log(struct btrfs_trans_handle *trans,
 		victim_parent = btrfs_iget_logging(parent_objectid, root);
 		if (IS_ERR(victim_parent)) {
 			kfree(victim_name.name);
-			return PTR_ERR(victim_parent);
+			ret = PTR_ERR(victim_parent);
+			btrfs_abort_transaction(trans, ret);
+			return ret;
 		}
 
 		inc_nlink(&inode->vfs_inode);
@@ -1200,6 +1249,7 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
 	search_key.offset = parent_objectid;
 	ret = btrfs_search_slot(NULL, root, &search_key, path, 0, 0);
 	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 		return ret;
 	} else if (ret == 0) {
 		/*
@@ -1237,7 +1287,9 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
 	di = btrfs_lookup_dir_index_item(trans, root, path, btrfs_ino(dir),
 					 ref_index, name, 0);
 	if (IS_ERR(di)) {
-		return PTR_ERR(di);
+		ret = PTR_ERR(di);
+		btrfs_abort_transaction(trans, ret);
+		return ret;
 	} else if (di) {
 		ret = drop_one_dir_item(trans, path, dir, di);
 		if (ret)
@@ -1327,8 +1379,10 @@ static int unlink_old_inode_refs(struct btrfs_trans_handle *trans,
 		ret = 0;
 		goto out;
 	}
-	if (ret < 0)
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 
 	eb = path->nodes[0];
 	ref_ptr = btrfs_item_ptr_offset(eb, path->slots[0]);
@@ -1340,12 +1394,18 @@ static int unlink_old_inode_refs(struct btrfs_trans_handle *trans,
 		if (key->type == BTRFS_INODE_EXTREF_KEY) {
 			ret = extref_get_fields(eb, ref_ptr, &name,
 						NULL, &parent_id);
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
+				goto out;
+			}
 		} else {
 			parent_id = key->offset;
 			ret = ref_get_fields(eb, ref_ptr, &name, NULL);
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
+				goto out;
+			}
 		}
-		if (ret)
-			goto out;
 
 		if (key->type == BTRFS_INODE_EXTREF_KEY)
 			ret = !!btrfs_find_name_in_ext_backref(log_eb, log_slot,
@@ -1361,6 +1421,7 @@ static int unlink_old_inode_refs(struct btrfs_trans_handle *trans,
 			if (IS_ERR(dir)) {
 				ret = PTR_ERR(dir);
 				kfree(name.name);
+				btrfs_abort_transaction(trans, ret);
 				goto out;
 			}
 			ret = unlink_inode_for_log_replay(trans, dir, inode, &name);
@@ -1435,6 +1496,8 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
 		ret = PTR_ERR(dir);
 		if (ret == -ENOENT)
 			ret = 0;
+		else
+			btrfs_abort_transaction(trans, ret);
 		dir = NULL;
 		goto out;
 	}
@@ -1442,6 +1505,7 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
 	inode = btrfs_iget_logging(inode_objectid, root);
 	if (IS_ERR(inode)) {
 		ret = PTR_ERR(inode);
+		btrfs_abort_transaction(trans, ret);
 		inode = NULL;
 		goto out;
 	}
@@ -1450,8 +1514,10 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
 		if (is_extref_item) {
 			ret = extref_get_fields(eb, ref_ptr, &name,
 						&ref_index, &parent_objectid);
-			if (ret)
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
+			}
 			/*
 			 * parent object can change from one array
 			 * item to another.
@@ -1476,19 +1542,24 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
 						 */
 						ret = 0;
 						goto next;
+					} else {
+						btrfs_abort_transaction(trans, ret);
 					}
 					goto out;
 				}
 			}
 		} else {
 			ret = ref_get_fields(eb, ref_ptr, &name, &ref_index);
-			if (ret)
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
+			}
 		}
 
 		ret = inode_in_dir(root, path, btrfs_ino(dir), btrfs_ino(inode),
 				   ref_index, &name);
 		if (ret < 0) {
+			btrfs_abort_transaction(trans, ret);
 			goto out;
 		} else if (ret == 0) {
 			/*
@@ -1509,12 +1580,16 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
 
 			/* insert our name */
 			ret = btrfs_add_link(trans, dir, inode, &name, 0, ref_index);
-			if (ret)
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
+			}
 
 			ret = btrfs_update_inode(trans, inode);
-			if (ret)
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
+			}
 		}
 		/* Else, ret == 1, we already have a perfect match, we're done. */
 
@@ -1786,8 +1861,11 @@ static noinline int link_to_fixup_dir(struct btrfs_trans_handle *trans,
 	struct inode *vfs_inode;
 
 	inode = btrfs_iget_logging(objectid, root);
-	if (IS_ERR(inode))
-		return PTR_ERR(inode);
+	if (IS_ERR(inode)) {
+		ret = PTR_ERR(inode);
+		btrfs_abort_transaction(trans, ret);
+		return ret;
+	}
 
 	vfs_inode = &inode->vfs_inode;
 	key.objectid = BTRFS_TREE_LOG_FIXUP_OBJECTID;
@@ -1805,6 +1883,8 @@ static noinline int link_to_fixup_dir(struct btrfs_trans_handle *trans,
 		ret = btrfs_update_inode(trans, inode);
 	} else if (ret == -EEXIST) {
 		ret = 0;
+	} else {
+		btrfs_abort_transaction(trans, ret);
 	}
 	iput(vfs_inode);
 
@@ -1911,19 +1991,26 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 	bool name_added = false;
 
 	dir = btrfs_iget_logging(key->objectid, root);
-	if (IS_ERR(dir))
-		return PTR_ERR(dir);
+	if (IS_ERR(dir)) {
+		ret = PTR_ERR(dir);
+		btrfs_abort_transaction(trans, ret);
+		return ret;
+	}
 
 	ret = read_alloc_one_name(eb, di + 1, btrfs_dir_name_len(eb, di), &name);
-	if (ret)
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 
 	log_flags = btrfs_dir_flags(eb, di);
 	btrfs_dir_item_key_to_cpu(eb, di, &log_key);
 	ret = btrfs_lookup_inode(trans, root, path, &log_key, 0);
 	btrfs_release_path(path);
-	if (ret < 0)
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 	exists = (ret == 0);
 	ret = 0;
 
@@ -1931,12 +2018,15 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 					   &name, 1);
 	if (IS_ERR(dir_dst_di)) {
 		ret = PTR_ERR(dir_dst_di);
+		btrfs_abort_transaction(trans, ret);
 		goto out;
 	} else if (dir_dst_di) {
 		ret = delete_conflicting_dir_entry(trans, dir, path, dir_dst_di,
 						   &log_key, log_flags, exists);
-		if (ret < 0)
+		if (ret < 0) {
+			btrfs_abort_transaction(trans, ret);
 			goto out;
+		}
 		dir_dst_matches = (ret == 1);
 	}
 
@@ -1947,12 +2037,15 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 						   &name, 1);
 	if (IS_ERR(index_dst_di)) {
 		ret = PTR_ERR(index_dst_di);
+		btrfs_abort_transaction(trans, ret);
 		goto out;
 	} else if (index_dst_di) {
 		ret = delete_conflicting_dir_entry(trans, dir, path, index_dst_di,
 						   &log_key, log_flags, exists);
-		if (ret < 0)
+		if (ret < 0) {
+			btrfs_abort_transaction(trans, ret);
 			goto out;
+		}
 		index_dst_matches = (ret == 1);
 	}
 
@@ -1973,6 +2066,7 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 	search_key.offset = key->objectid;
 	ret = backref_in_log(root->log_root, &search_key, 0, &name);
 	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 	        goto out;
 	} else if (ret) {
 	        /* The dentry will be added later. */
@@ -1986,6 +2080,7 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 	search_key.offset = key->objectid;
 	ret = backref_in_log(root->log_root, &search_key, key->objectid, &name);
 	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
 	} else if (ret) {
 		/* The dentry will be added later. */
@@ -1996,8 +2091,10 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 	btrfs_release_path(path);
 	ret = insert_one_name(trans, root, key->objectid, key->offset,
 			      &name, &log_key);
-	if (ret && ret != -ENOENT && ret != -EEXIST)
+	if (ret && ret != -ENOENT && ret != -EEXIST) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 	if (!ret)
 		name_added = true;
 	update_size = false;
@@ -2007,6 +2104,8 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 	if (!ret && update_size) {
 		btrfs_i_size_write(dir, dir->vfs_inode.i_size + name.len * 2);
 		ret = btrfs_update_inode(trans, dir);
+		if (ret)
+			btrfs_abort_transaction(trans, ret);
 	}
 	kfree(name.name);
 	iput(&dir->vfs_inode);
@@ -2064,8 +2163,10 @@ static noinline int replay_one_dir_item(struct btrfs_trans_handle *trans,
 		struct btrfs_key di_key;
 
 		fixup_path = btrfs_alloc_path();
-		if (!fixup_path)
+		if (!fixup_path) {
+			btrfs_abort_transaction(trans, -ENOMEM);
 			return -ENOMEM;
+		}
 
 		btrfs_dir_item_key_to_cpu(eb, di, &di_key);
 		ret = link_to_fixup_dir(trans, root, fixup_path, di_key.objectid);
@@ -2190,8 +2291,10 @@ static noinline int check_item_in_log(struct btrfs_trans_handle *trans,
 	slot = path->slots[0];
 	di = btrfs_item_ptr(eb, slot, struct btrfs_dir_item);
 	ret = read_alloc_one_name(eb, di + 1, btrfs_dir_name_len(eb, di), &name);
-	if (ret)
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 
 	if (log) {
 		struct btrfs_dir_item *log_di;
@@ -2201,6 +2304,7 @@ static noinline int check_item_in_log(struct btrfs_trans_handle *trans,
 						     dir_key->offset, &name, 0);
 		if (IS_ERR(log_di)) {
 			ret = PTR_ERR(log_di);
+			btrfs_abort_transaction(trans, ret);
 			goto out;
 		} else if (log_di) {
 			/* The dentry exists in the log, we have nothing to do. */
@@ -2216,6 +2320,7 @@ static noinline int check_item_in_log(struct btrfs_trans_handle *trans,
 	if (IS_ERR(inode)) {
 		ret = PTR_ERR(inode);
 		inode = NULL;
+		btrfs_abort_transaction(trans, ret);
 		goto out;
 	}
 
@@ -2252,16 +2357,20 @@ static int replay_xattr_deletes(struct btrfs_trans_handle *trans,
 	int ret;
 
 	log_path = btrfs_alloc_path();
-	if (!log_path)
+	if (!log_path) {
+		btrfs_abort_transaction(trans, -ENOMEM);
 		return -ENOMEM;
+	}
 
 	search_key.objectid = ino;
 	search_key.type = BTRFS_XATTR_ITEM_KEY;
 	search_key.offset = 0;
 again:
 	ret = btrfs_search_slot(NULL, root, &search_key, path, 0, 0);
-	if (ret < 0)
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 		goto out;
+	}
 process_leaf:
 	nritems = btrfs_header_nritems(path->nodes[0]);
 	for (i = path->slots[0]; i < nritems; i++) {
@@ -2289,6 +2398,7 @@ static int replay_xattr_deletes(struct btrfs_trans_handle *trans,
 			name = kmalloc(name_len, GFP_NOFS);
 			if (!name) {
 				ret = -ENOMEM;
+				btrfs_abort_transaction(trans, ret);
 				goto out;
 			}
 			read_extent_buffer(path->nodes[0], name,
@@ -2305,13 +2415,16 @@ static int replay_xattr_deletes(struct btrfs_trans_handle *trans,
 				kfree(name);
 				if (IS_ERR(di)) {
 					ret = PTR_ERR(di);
+					btrfs_abort_transaction(trans, ret);
 					goto out;
 				}
 				ASSERT(di);
 				ret = btrfs_delete_one_dir_name(trans, root,
 								path, di);
-				if (ret)
+				if (ret) {
+					btrfs_abort_transaction(trans, ret);
 					goto out;
+				}
 				btrfs_release_path(path);
 				search_key = key;
 				goto again;
@@ -2319,6 +2432,7 @@ static int replay_xattr_deletes(struct btrfs_trans_handle *trans,
 			kfree(name);
 			if (IS_ERR(log_di)) {
 				ret = PTR_ERR(log_di);
+				btrfs_abort_transaction(trans, ret);
 				goto out;
 			}
 			cur += this_len;
@@ -2330,6 +2444,8 @@ static int replay_xattr_deletes(struct btrfs_trans_handle *trans,
 		ret = 0;
 	else if (ret == 0)
 		goto process_leaf;
+	else
+		btrfs_abort_transaction(trans, ret);
 out:
 	btrfs_free_path(log_path);
 	btrfs_release_path(path);
@@ -2364,8 +2480,10 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,
 	dir_key.objectid = dirid;
 	dir_key.type = BTRFS_DIR_INDEX_KEY;
 	log_path = btrfs_alloc_path();
-	if (!log_path)
+	if (!log_path) {
+		btrfs_abort_transaction(trans, -ENOMEM);
 		return -ENOMEM;
+	}
 
 	dir = btrfs_iget_logging(dirid, root);
 	/*
@@ -2377,6 +2495,8 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,
 		ret = PTR_ERR(dir);
 		if (ret == -ENOENT)
 			ret = 0;
+		else
+			btrfs_abort_transaction(trans, ret);
 		return ret;
 	}
 
@@ -2388,10 +2508,12 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,
 		else {
 			ret = find_dir_range(log, path, dirid,
 					     &range_start, &range_end);
-			if (ret < 0)
+			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
-			else if (ret > 0)
+			} else if (ret > 0) {
 				break;
+			}
 		}
 
 		dir_key.offset = range_start;
@@ -2399,16 +2521,20 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,
 			int nritems;
 			ret = btrfs_search_slot(NULL, root, &dir_key, path,
 						0, 0);
-			if (ret < 0)
+			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
 				goto out;
+			}
 
 			nritems = btrfs_header_nritems(path->nodes[0]);
 			if (path->slots[0] >= nritems) {
 				ret = btrfs_next_leaf(root, path);
-				if (ret == 1)
+				if (ret == 1) {
 					break;
-				else if (ret < 0)
+				} else if (ret < 0) {
+					btrfs_abort_transaction(trans, ret);
 					goto out;
+				}
 			}
 			btrfs_item_key_to_cpu(path->nodes[0], &found_key,
 					      path->slots[0]);
@@ -2470,8 +2596,10 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 	int ret;
 
 	ret = btrfs_read_extent_buffer(eb, &check);
-	if (ret)
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
 		return ret;
+	}
 
 	level = btrfs_header_level(eb);
 
@@ -2479,8 +2607,10 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 		return 0;
 
 	path = btrfs_alloc_path();
-	if (!path)
+	if (!path) {
+		btrfs_abort_transaction(trans, -ENOMEM);
 		return -ENOMEM;
+	}
 
 	nritems = btrfs_header_nritems(eb);
 	for (i = 0; i < nritems; i++) {
@@ -2538,6 +2668,7 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 				inode = btrfs_iget_logging(key.objectid, root);
 				if (IS_ERR(inode)) {
 					ret = PTR_ERR(inode);
+					btrfs_abort_transaction(trans, ret);
 					break;
 				}
 				from = ALIGN(i_size_read(&inode->vfs_inode),
@@ -2546,11 +2677,15 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 				drop_args.end = (u64)-1;
 				drop_args.drop_cache = true;
 				ret = btrfs_drop_extents(trans, root, inode,  &drop_args);
-				if (!ret) {
+				if (ret) {
+					btrfs_abort_transaction(trans, ret);
+				} else {
 					inode_sub_bytes(&inode->vfs_inode,
 							drop_args.bytes_found);
 					/* Update the inode's nbytes. */
 					ret = btrfs_update_inode(trans, inode);
+					if (ret)
+						btrfs_abort_transaction(trans, ret);
 				}
 				iput(&inode->vfs_inode);
 				if (ret)
-- 
2.47.2

[PATCH 07/10] btrfs: exit early when replaying hole file extent item from a log tree

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

At replay_one_extent(), we can jump to the code that updates the file
extent range and updates the inode when processing a file extent item that
represents a hole and we don't have the NO_HOLES feature enabled. This
helps reduce the high indentation level by one in replay_one_extent() and
avoid splitting some lines to make the code easier to read.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 17af4ff51479..158959c8cc11 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -732,6 +732,9 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 
 	if (found_type == BTRFS_FILE_EXTENT_REG ||
 	    found_type == BTRFS_FILE_EXTENT_PREALLOC) {
+		u64 csum_start;
+		u64 csum_end;
+		LIST_HEAD(ordered_sums);
 		u64 offset;
 		unsigned long dest_offset;
 		struct btrfs_key ins;
@@ -751,6 +754,17 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 		copy_extent_buffer(path->nodes[0], eb, dest_offset,
 				(unsigned long)item,  sizeof(*item));
 
+		/*
+		 * We have an explicit hole and NO_HOLES is not enabled. We have
+		 * added the hole file extent item to the subvolume tree, so we
+		 * don't have anything else to do other than update the file
+		 * extent item range and update the inode item.
+		 */
+		if (btrfs_file_extent_disk_bytenr(eb, item) == 0) {
+			btrfs_release_path(path);
+			goto update_inode;
+		}
+
 		ins.objectid = btrfs_file_extent_disk_bytenr(eb, item);
 		ins.type = BTRFS_EXTENT_ITEM_KEY;
 		ins.offset = btrfs_file_extent_disk_num_bytes(eb, item);
@@ -772,148 +786,125 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 			goto out;
 		}
 
-		if (ins.objectid > 0) {
-			u64 csum_start;
-			u64 csum_end;
-			LIST_HEAD(ordered_sums);
-
-			/*
-			 * is this extent already allocated in the extent
-			 * allocation tree?  If so, just add a reference
-			 */
-			ret = btrfs_lookup_data_extent(fs_info, ins.objectid,
-						ins.offset);
-			if (ret < 0) {
+		/*
+		 * Is this extent already allocated in the extent tree?
+		 * If so, just add a reference.
+		 */
+		ret = btrfs_lookup_data_extent(fs_info, ins.objectid, ins.offset);
+		if (ret < 0) {
+			btrfs_abort_transaction(trans, ret);
+			goto out;
+		} else if (ret == 0) {
+			struct btrfs_ref ref = {
+				.action = BTRFS_ADD_DELAYED_REF,
+				.bytenr = ins.objectid,
+				.num_bytes = ins.offset,
+				.owning_root = btrfs_root_id(root),
+				.ref_root = btrfs_root_id(root),
+			};
+
+			btrfs_init_data_ref(&ref, key->objectid, offset, 0, false);
+			ret = btrfs_inc_extent_ref(trans, &ref);
+			if (ret) {
 				btrfs_abort_transaction(trans, ret);
 				goto out;
-			} else if (ret == 0) {
-				struct btrfs_ref ref = {
-					.action = BTRFS_ADD_DELAYED_REF,
-					.bytenr = ins.objectid,
-					.num_bytes = ins.offset,
-					.owning_root = btrfs_root_id(root),
-					.ref_root = btrfs_root_id(root),
-				};
-				btrfs_init_data_ref(&ref, key->objectid, offset,
-						    0, false);
-				ret = btrfs_inc_extent_ref(trans, &ref);
-				if (ret) {
-					btrfs_abort_transaction(trans, ret);
-					goto out;
-				}
-			} else {
-				/*
-				 * insert the extent pointer in the extent
-				 * allocation tree
-				 */
-				ret = btrfs_alloc_logged_file_extent(trans,
-						btrfs_root_id(root),
-						key->objectid, offset, &ins);
-				if (ret) {
-					btrfs_abort_transaction(trans, ret);
-					goto out;
-				}
-			}
-			btrfs_release_path(path);
-
-			if (btrfs_file_extent_compression(eb, item)) {
-				csum_start = ins.objectid;
-				csum_end = csum_start + ins.offset;
-			} else {
-				csum_start = ins.objectid +
-					btrfs_file_extent_offset(eb, item);
-				csum_end = csum_start +
-					btrfs_file_extent_num_bytes(eb, item);
 			}
-
-			ret = btrfs_lookup_csums_list(root->log_root,
-						csum_start, csum_end - 1,
-						&ordered_sums, false);
-			if (ret < 0) {
+		} else {
+			/* Insert the extent pointer in the extent tree. */
+			ret = btrfs_alloc_logged_file_extent(trans, btrfs_root_id(root),
+							     key->objectid, offset, &ins);
+			if (ret) {
 				btrfs_abort_transaction(trans, ret);
 				goto out;
 			}
-			ret = 0;
-			/*
-			 * Now delete all existing cums in the csum root that
-			 * cover our range. We do this because we can have an
-			 * extent that is completely referenced by one file
-			 * extent item and partially referenced by another
-			 * file extent item (like after using the clone or
-			 * extent_same ioctls). In this case if we end up doing
-			 * the replay of the one that partially references the
-			 * extent first, and we do not do the csum deletion
-			 * below, we can get 2 csum items in the csum tree that
-			 * overlap each other. For example, imagine our log has
-			 * the two following file extent items:
-			 *
-			 * key (257 EXTENT_DATA 409600)
-			 *     extent data disk byte 12845056 nr 102400
-			 *     extent data offset 20480 nr 20480 ram 102400
-			 *
-			 * key (257 EXTENT_DATA 819200)
-			 *     extent data disk byte 12845056 nr 102400
-			 *     extent data offset 0 nr 102400 ram 102400
-			 *
-			 * Where the second one fully references the 100K extent
-			 * that starts at disk byte 12845056, and the log tree
-			 * has a single csum item that covers the entire range
-			 * of the extent:
-			 *
-			 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
-			 *
-			 * After the first file extent item is replayed, the
-			 * csum tree gets the following csum item:
-			 *
-			 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
-			 *
-			 * Which covers the 20K sub-range starting at offset 20K
-			 * of our extent. Now when we replay the second file
-			 * extent item, if we do not delete existing csum items
-			 * that cover any of its blocks, we end up getting two
-			 * csum items in our csum tree that overlap each other:
-			 *
-			 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
-			 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
-			 *
-			 * Which is a problem, because after this anyone trying
-			 * to lookup up for the checksum of any block of our
-			 * extent starting at an offset of 40K or higher, will
-			 * end up looking at the second csum item only, which
-			 * does not contain the checksum for any block starting
-			 * at offset 40K or higher of our extent.
-			 */
-			while (!list_empty(&ordered_sums)) {
-				struct btrfs_ordered_sum *sums;
-				struct btrfs_root *csum_root;
-
-				sums = list_first_entry(&ordered_sums,
-							struct btrfs_ordered_sum,
-							list);
-				csum_root = btrfs_csum_root(fs_info,
-							    sums->logical);
-				if (!ret) {
-					ret = btrfs_del_csums(trans, csum_root,
-							      sums->logical,
-							      sums->len);
-					if (ret)
-						btrfs_abort_transaction(trans, ret);
-				}
-				if (!ret) {
-					ret = btrfs_csum_file_blocks(trans,
-								     csum_root,
-								     sums);
-					if (ret)
-						btrfs_abort_transaction(trans, ret);
-				}
-				list_del(&sums->list);
-				kfree(sums);
-			}
-			if (ret)
-				goto out;
+		}
+
+		btrfs_release_path(path);
+
+		if (btrfs_file_extent_compression(eb, item)) {
+			csum_start = ins.objectid;
+			csum_end = csum_start + ins.offset;
 		} else {
-			btrfs_release_path(path);
+			csum_start = ins.objectid + btrfs_file_extent_offset(eb, item);
+			csum_end = csum_start + btrfs_file_extent_num_bytes(eb, item);
+		}
+
+		ret = btrfs_lookup_csums_list(root->log_root, csum_start, csum_end - 1,
+					      &ordered_sums, false);
+		if (ret < 0) {
+			btrfs_abort_transaction(trans, ret);
+			goto out;
 		}
+		ret = 0;
+		/*
+		 * Now delete all existing cums in the csum root that cover our
+		 * range. We do this because we can have an extent that is
+		 * completely referenced by one file extent item and partially
+		 * referenced by another file extent item (like after using the
+		 * clone or extent_same ioctls). In this case if we end up doing
+		 * the replay of the one that partially references the extent
+		 * first, and we do not do the csum deletion below, we can get 2
+		 * csum items in the csum tree that overlap each other. For
+		 * example, imagine our log has the two following file extent items:
+		 *
+		 * key (257 EXTENT_DATA 409600)
+		 *     extent data disk byte 12845056 nr 102400
+		 *     extent data offset 20480 nr 20480 ram 102400
+		 *
+		 * key (257 EXTENT_DATA 819200)
+		 *     extent data disk byte 12845056 nr 102400
+		 *     extent data offset 0 nr 102400 ram 102400
+		 *
+		 * Where the second one fully references the 100K extent that
+		 * starts at disk byte 12845056, and the log tree has a single
+		 * csum item that covers the entire range of the extent:
+		 *
+		 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
+		 *
+		 * After the first file extent item is replayed, the csum tree
+		 * gets the following csum item:
+		 *
+		 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
+		 *
+		 * Which covers the 20K sub-range starting at offset 20K of our
+		 * extent. Now when we replay the second file extent item, if we
+		 * do not delete existing csum items that cover any of its
+		 * blocks, we end up getting two csum items in our csum tree
+		 * that overlap each other:
+		 *
+		 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
+		 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
+		 *
+		 * Which is a problem, because after this anyone trying to
+		 * lookup up for the checksum of any block of our extent
+		 * starting at an offset of 40K or higher, will end up looking
+		 * at the second csum item only, which does not contain the
+		 * checksum for any block starting at offset 40K or higher of
+		 * our extent.
+		 */
+		while (!list_empty(&ordered_sums)) {
+			struct btrfs_ordered_sum *sums;
+			struct btrfs_root *csum_root;
+
+			sums = list_first_entry(&ordered_sums,
+						struct btrfs_ordered_sum, list);
+			csum_root = btrfs_csum_root(fs_info, sums->logical);
+			if (!ret) {
+				ret = btrfs_del_csums(trans, csum_root, sums->logical,
+						      sums->len);
+				if (ret)
+					btrfs_abort_transaction(trans, ret);
+			}
+			if (!ret) {
+				ret = btrfs_csum_file_blocks(trans, csum_root, sums);
+				if (ret)
+					btrfs_abort_transaction(trans, ret);
+			}
+			list_del(&sums->list);
+			kfree(sums);
+		}
+		if (ret)
+			goto out;
 	} else if (found_type == BTRFS_FILE_EXTENT_INLINE) {
 		/* inline extents are easy, we just overwrite them */
 		ret = overwrite_item(trans, root, path, eb, slot, key);
@@ -921,13 +912,13 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 			goto out;
 	}
 
+update_inode:
 	ret = btrfs_inode_set_file_extent_range(inode, start, extent_end - start);
 	if (ret) {
 		btrfs_abort_transaction(trans, ret);
 		goto out;
 	}
 
-update_inode:
 	btrfs_update_inode_bytes(inode, nbytes, drop_args.bytes_found);
 	ret = btrfs_update_inode(trans, inode);
 	if (ret)
-- 
2.47.2

[PATCH 08/10] btrfs: process inline extent earlier in replay_one_extent()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Instead of having an if statement to check for regular and prealloc
extents first, process them in a block, and then following with an else
statement to check for an inline extent, check for an inline extent first,
process it and jump to the 'update_inode' label, allowing us to avoid
having the code for processing regular and prealloc extents inside a
block, reducing the high indentantion level by one and making the coder
easier to read and avoid line splittings too.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 158959c8cc11..8817dfa5b353 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -653,6 +653,12 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 	u64 extent_end;
 	u64 start = key->offset;
 	u64 nbytes = 0;
+	u64 csum_start;
+	u64 csum_end;
+	LIST_HEAD(ordered_sums);
+	u64 offset;
+	unsigned long dest_offset;
+	struct btrfs_key ins;
 	struct btrfs_file_extent_item *item;
 	struct btrfs_inode *inode = NULL;
 	unsigned long size;
@@ -730,187 +736,180 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 		goto out;
 	}
 
-	if (found_type == BTRFS_FILE_EXTENT_REG ||
-	    found_type == BTRFS_FILE_EXTENT_PREALLOC) {
-		u64 csum_start;
-		u64 csum_end;
-		LIST_HEAD(ordered_sums);
-		u64 offset;
-		unsigned long dest_offset;
-		struct btrfs_key ins;
+	if (found_type == BTRFS_FILE_EXTENT_INLINE) {
+		/* inline extents are easy, we just overwrite them */
+		ret = overwrite_item(trans, root, path, eb, slot, key);
+		if (ret)
+			goto out;
+		goto update_inode;
+	}
 
-		if (btrfs_file_extent_disk_bytenr(eb, item) == 0 &&
-		    btrfs_fs_incompat(fs_info, NO_HOLES))
-			goto update_inode;
+	/*
+	 * If not an inline extent, it can only be a regular or prealloc one.
+	 * We have checked that above and returned -EUCLEAN if not.
+	 */
 
-		ret = btrfs_insert_empty_item(trans, root, path, key,
-					      sizeof(*item));
-		if (ret) {
-			btrfs_abort_transaction(trans, ret);
-			goto out;
-		}
-		dest_offset = btrfs_item_ptr_offset(path->nodes[0],
-						    path->slots[0]);
-		copy_extent_buffer(path->nodes[0], eb, dest_offset,
-				(unsigned long)item,  sizeof(*item));
+	/* A hole and NO_HOLES feature enabled, nothing else to do. */
+	if (btrfs_file_extent_disk_bytenr(eb, item) == 0 &&
+	    btrfs_fs_incompat(fs_info, NO_HOLES))
+		goto update_inode;
 
-		/*
-		 * We have an explicit hole and NO_HOLES is not enabled. We have
-		 * added the hole file extent item to the subvolume tree, so we
-		 * don't have anything else to do other than update the file
-		 * extent item range and update the inode item.
-		 */
-		if (btrfs_file_extent_disk_bytenr(eb, item) == 0) {
-			btrfs_release_path(path);
-			goto update_inode;
-		}
+	ret = btrfs_insert_empty_item(trans, root, path, key, sizeof(*item));
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
+		goto out;
+	}
+	dest_offset = btrfs_item_ptr_offset(path->nodes[0], path->slots[0]);
+	copy_extent_buffer(path->nodes[0], eb, dest_offset, (unsigned long)item,
+			   sizeof(*item));
 
-		ins.objectid = btrfs_file_extent_disk_bytenr(eb, item);
-		ins.type = BTRFS_EXTENT_ITEM_KEY;
-		ins.offset = btrfs_file_extent_disk_num_bytes(eb, item);
-		offset = key->offset - btrfs_file_extent_offset(eb, item);
+	/*
+	 * We have an explicit hole and NO_HOLES is not enabled. We have added
+	 * the hole file extent item to the subvolume tree, so we don't have
+	 * anything else to do other than update the file extent item range and
+	 * update the inode item.
+	 */
+	if (btrfs_file_extent_disk_bytenr(eb, item) == 0) {
+		btrfs_release_path(path);
+		goto update_inode;
+	}
 
-		/*
-		 * Manually record dirty extent, as here we did a shallow
-		 * file extent item copy and skip normal backref update,
-		 * but modifying extent tree all by ourselves.
-		 * So need to manually record dirty extent for qgroup,
-		 * as the owner of the file extent changed from log tree
-		 * (doesn't affect qgroup) to fs/file tree(affects qgroup)
-		 */
-		ret = btrfs_qgroup_trace_extent(trans,
-				btrfs_file_extent_disk_bytenr(eb, item),
-				btrfs_file_extent_disk_num_bytes(eb, item));
-		if (ret < 0) {
+	ins.objectid = btrfs_file_extent_disk_bytenr(eb, item);
+	ins.type = BTRFS_EXTENT_ITEM_KEY;
+	ins.offset = btrfs_file_extent_disk_num_bytes(eb, item);
+	offset = key->offset - btrfs_file_extent_offset(eb, item);
+
+	/*
+	 * Manually record dirty extent, as here we did a shallow file extent
+	 * item copy and skip normal backref update, but modifying extent tree
+	 * all by ourselves. So need to manually record dirty extent for qgroup,
+	 * as the owner of the file extent changed from log tree (doesn't affect
+	 * qgroup) to fs/file tree (affects qgroup).
+	 */
+	ret = btrfs_qgroup_trace_extent(trans,
+					btrfs_file_extent_disk_bytenr(eb, item),
+					btrfs_file_extent_disk_num_bytes(eb, item));
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
+		goto out;
+	}
+
+	/*
+	 * Is this extent already allocated in the extent tree?
+	 * If so, just add a reference.
+	 */
+	ret = btrfs_lookup_data_extent(fs_info, ins.objectid, ins.offset);
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
+		goto out;
+	} else if (ret == 0) {
+		struct btrfs_ref ref = {
+			.action = BTRFS_ADD_DELAYED_REF,
+			.bytenr = ins.objectid,
+			.num_bytes = ins.offset,
+			.owning_root = btrfs_root_id(root),
+			.ref_root = btrfs_root_id(root),
+		};
+
+		btrfs_init_data_ref(&ref, key->objectid, offset, 0, false);
+		ret = btrfs_inc_extent_ref(trans, &ref);
+		if (ret) {
 			btrfs_abort_transaction(trans, ret);
 			goto out;
 		}
-
-		/*
-		 * Is this extent already allocated in the extent tree?
-		 * If so, just add a reference.
-		 */
-		ret = btrfs_lookup_data_extent(fs_info, ins.objectid, ins.offset);
-		if (ret < 0) {
+	} else {
+		/* Insert the extent pointer in the extent tree. */
+		ret = btrfs_alloc_logged_file_extent(trans, btrfs_root_id(root),
+						     key->objectid, offset, &ins);
+		if (ret) {
 			btrfs_abort_transaction(trans, ret);
 			goto out;
-		} else if (ret == 0) {
-			struct btrfs_ref ref = {
-				.action = BTRFS_ADD_DELAYED_REF,
-				.bytenr = ins.objectid,
-				.num_bytes = ins.offset,
-				.owning_root = btrfs_root_id(root),
-				.ref_root = btrfs_root_id(root),
-			};
-
-			btrfs_init_data_ref(&ref, key->objectid, offset, 0, false);
-			ret = btrfs_inc_extent_ref(trans, &ref);
-			if (ret) {
-				btrfs_abort_transaction(trans, ret);
-				goto out;
-			}
-		} else {
-			/* Insert the extent pointer in the extent tree. */
-			ret = btrfs_alloc_logged_file_extent(trans, btrfs_root_id(root),
-							     key->objectid, offset, &ins);
-			if (ret) {
-				btrfs_abort_transaction(trans, ret);
-				goto out;
-			}
 		}
+	}
 
-		btrfs_release_path(path);
+	btrfs_release_path(path);
 
-		if (btrfs_file_extent_compression(eb, item)) {
-			csum_start = ins.objectid;
-			csum_end = csum_start + ins.offset;
-		} else {
-			csum_start = ins.objectid + btrfs_file_extent_offset(eb, item);
-			csum_end = csum_start + btrfs_file_extent_num_bytes(eb, item);
-		}
+	if (btrfs_file_extent_compression(eb, item)) {
+		csum_start = ins.objectid;
+		csum_end = csum_start + ins.offset;
+	} else {
+		csum_start = ins.objectid + btrfs_file_extent_offset(eb, item);
+		csum_end = csum_start + btrfs_file_extent_num_bytes(eb, item);
+	}
 
-		ret = btrfs_lookup_csums_list(root->log_root, csum_start, csum_end - 1,
-					      &ordered_sums, false);
-		if (ret < 0) {
-			btrfs_abort_transaction(trans, ret);
-			goto out;
-		}
-		ret = 0;
-		/*
-		 * Now delete all existing cums in the csum root that cover our
-		 * range. We do this because we can have an extent that is
-		 * completely referenced by one file extent item and partially
-		 * referenced by another file extent item (like after using the
-		 * clone or extent_same ioctls). In this case if we end up doing
-		 * the replay of the one that partially references the extent
-		 * first, and we do not do the csum deletion below, we can get 2
-		 * csum items in the csum tree that overlap each other. For
-		 * example, imagine our log has the two following file extent items:
-		 *
-		 * key (257 EXTENT_DATA 409600)
-		 *     extent data disk byte 12845056 nr 102400
-		 *     extent data offset 20480 nr 20480 ram 102400
-		 *
-		 * key (257 EXTENT_DATA 819200)
-		 *     extent data disk byte 12845056 nr 102400
-		 *     extent data offset 0 nr 102400 ram 102400
-		 *
-		 * Where the second one fully references the 100K extent that
-		 * starts at disk byte 12845056, and the log tree has a single
-		 * csum item that covers the entire range of the extent:
-		 *
-		 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
-		 *
-		 * After the first file extent item is replayed, the csum tree
-		 * gets the following csum item:
-		 *
-		 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
-		 *
-		 * Which covers the 20K sub-range starting at offset 20K of our
-		 * extent. Now when we replay the second file extent item, if we
-		 * do not delete existing csum items that cover any of its
-		 * blocks, we end up getting two csum items in our csum tree
-		 * that overlap each other:
-		 *
-		 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
-		 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
-		 *
-		 * Which is a problem, because after this anyone trying to
-		 * lookup up for the checksum of any block of our extent
-		 * starting at an offset of 40K or higher, will end up looking
-		 * at the second csum item only, which does not contain the
-		 * checksum for any block starting at offset 40K or higher of
-		 * our extent.
-		 */
-		while (!list_empty(&ordered_sums)) {
-			struct btrfs_ordered_sum *sums;
-			struct btrfs_root *csum_root;
+	ret = btrfs_lookup_csums_list(root->log_root, csum_start, csum_end - 1,
+				      &ordered_sums, false);
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
+		goto out;
+	}
+	ret = 0;
+	/*
+	 * Now delete all existing cums in the csum root that cover our range.
+	 * We do this because we can have an extent that is completely
+	 * referenced by one file extent item and partially referenced by
+	 * another file extent item (like after using the clone or extent_same
+	 * ioctls). In this case if we end up doing the replay of the one that
+	 * partially references the extent first, and we do not do the csum
+	 * deletion below, we can get 2 csum items in the csum tree that overlap
+	 * each other. For example, imagine our log has the two following file
+	 * extent items:
+	 *
+	 * key (257 EXTENT_DATA 409600)
+	 *     extent data disk byte 12845056 nr 102400
+	 *     extent data offset 20480 nr 20480 ram 102400
+	 *
+	 * key (257 EXTENT_DATA 819200)
+	 *     extent data disk byte 12845056 nr 102400
+	 *     extent data offset 0 nr 102400 ram 102400
+	 *
+	 * Where the second one fully references the 100K extent that starts at
+	 * disk byte 12845056, and the log tree has a single csum item that
+	 * covers the entire range of the extent:
+	 *
+	 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
+	 *
+	 * After the first file extent item is replayed, the csum tree gets the
+	 * following csum item:
+	 *
+	 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
+	 *
+	 * Which covers the 20K sub-range starting at offset 20K of our extent.
+	 * Now when we replay the second file extent item, if we do not delete
+	 * existing csum items that cover any of its blocks, we end up getting
+	 * two csum items in our csum tree that overlap each other:
+	 *
+	 * key (EXTENT_CSUM EXTENT_CSUM 12845056) itemsize 100
+	 * key (EXTENT_CSUM EXTENT_CSUM 12865536) itemsize 20
+	 *
+	 * Which is a problem, because after this anyone trying to lookup for
+	 * the checksum of any block of our extent starting at an offset of 40K
+	 * or higher, will end up looking at the second csum item only, which
+	 * does not contain the checksum for any block starting at offset 40K or
+	 * higher of our extent.
+	 */
+	while (!list_empty(&ordered_sums)) {
+		struct btrfs_ordered_sum *sums;
+		struct btrfs_root *csum_root;
 
-			sums = list_first_entry(&ordered_sums,
-						struct btrfs_ordered_sum, list);
-			csum_root = btrfs_csum_root(fs_info, sums->logical);
-			if (!ret) {
-				ret = btrfs_del_csums(trans, csum_root, sums->logical,
-						      sums->len);
-				if (ret)
-					btrfs_abort_transaction(trans, ret);
-			}
-			if (!ret) {
-				ret = btrfs_csum_file_blocks(trans, csum_root, sums);
-				if (ret)
-					btrfs_abort_transaction(trans, ret);
-			}
-			list_del(&sums->list);
-			kfree(sums);
+		sums = list_first_entry(&ordered_sums, struct btrfs_ordered_sum, list);
+		csum_root = btrfs_csum_root(fs_info, sums->logical);
+		if (!ret) {
+			ret = btrfs_del_csums(trans, csum_root, sums->logical,
+					      sums->len);
+			if (ret)
+				btrfs_abort_transaction(trans, ret);
 		}
-		if (ret)
-			goto out;
-	} else if (found_type == BTRFS_FILE_EXTENT_INLINE) {
-		/* inline extents are easy, we just overwrite them */
-		ret = overwrite_item(trans, root, path, eb, slot, key);
-		if (ret)
-			goto out;
+		if (!ret) {
+			ret = btrfs_csum_file_blocks(trans, csum_root, sums);
+			if (ret)
+				btrfs_abort_transaction(trans, ret);
+		}
+		list_del(&sums->list);
+		kfree(sums);
 	}
+	if (ret)
+		goto out;
 
 update_inode:
 	ret = btrfs_inode_set_file_extent_range(inode, start, extent_end - start);
-- 
2.47.2

[PATCH 09/10] btrfs: use local key variable to pass arguments in replay_one_extent()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Instead of extracting again the disk_bytenr and disk_num_bytes values from
the file extent item to pass to btrfs_qgroup_trace_extent(), use the key
local variable 'ins' which already has those values, reducing the size of
the source code.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 8817dfa5b353..85b623850a57 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -786,9 +786,7 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 	 * as the owner of the file extent changed from log tree (doesn't affect
 	 * qgroup) to fs/file tree (affects qgroup).
 	 */
-	ret = btrfs_qgroup_trace_extent(trans,
-					btrfs_file_extent_disk_bytenr(eb, item),
-					btrfs_file_extent_disk_num_bytes(eb, item));
+	ret = btrfs_qgroup_trace_extent(trans, ins.objectid, ins.offset);
 	if (ret < 0) {
 		btrfs_abort_transaction(trans, ret);
 		goto out;
-- 
2.47.2

[PATCH 10/10] btrfs: collapse unaccount_log_buffer() into clean_log_buffer()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There's one only one caller of unaccount_log_buffer() and both this
function and the caller are short, so move its code into the caller.

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

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 85b623850a57..43a96fb27bce 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2724,35 +2724,11 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
 	return ret;
 }
 
-/*
- * Correctly adjust the reserved bytes occupied by a log tree extent buffer
- */
-static int unaccount_log_buffer(struct btrfs_fs_info *fs_info, u64 start)
-{
-	struct btrfs_block_group *cache;
-
-	cache = btrfs_lookup_block_group(fs_info, start);
-	if (!cache) {
-		btrfs_err(fs_info, "unable to find block group for %llu", start);
-		return -ENOENT;
-	}
-
-	spin_lock(&cache->space_info->lock);
-	spin_lock(&cache->lock);
-	cache->reserved -= fs_info->nodesize;
-	cache->space_info->bytes_reserved -= fs_info->nodesize;
-	spin_unlock(&cache->lock);
-	spin_unlock(&cache->space_info->lock);
-
-	btrfs_put_block_group(cache);
-
-	return 0;
-}
-
 static int clean_log_buffer(struct btrfs_trans_handle *trans,
 			    struct extent_buffer *eb)
 {
-	int ret;
+	struct btrfs_fs_info *fs_info = eb->fs_info;
+	struct btrfs_block_group *bg;
 
 	btrfs_tree_lock(eb);
 	btrfs_clear_buffer_dirty(trans, eb);
@@ -2760,16 +2736,31 @@ static int clean_log_buffer(struct btrfs_trans_handle *trans,
 	btrfs_tree_unlock(eb);
 
 	if (trans) {
+		int ret;
+
 		ret = btrfs_pin_reserved_extent(trans, eb);
 		if (ret)
 			btrfs_abort_transaction(trans, ret);
 		return ret;
 	}
 
-	ret = unaccount_log_buffer(eb->fs_info, eb->start);
-	if (ret)
-		btrfs_handle_fs_error(eb->fs_info, ret, NULL);
-	return ret;
+	bg = btrfs_lookup_block_group(fs_info, eb->start);
+	if (!bg) {
+		btrfs_err(fs_info, "unable to find block group for %llu", eb->start);
+		btrfs_handle_fs_error(fs_info, -ENOENT, NULL);
+		return -ENOENT;
+	}
+
+	spin_lock(&bg->space_info->lock);
+	spin_lock(&bg->lock);
+	bg->reserved -= fs_info->nodesize;
+	bg->space_info->bytes_reserved -= fs_info->nodesize;
+	spin_unlock(&bg->lock);
+	spin_unlock(&bg->space_info->lock);
+
+	btrfs_put_block_group(bg);
+
+	return 0;
 }
 
 static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
-- 
2.47.2

Re: [PATCH 00/10] btrfs: improve error reporting for log tree replay

[Qu Wenruo]

在 2025/7/22 02:46, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Most errors that happen during log replay or destroying a log tree are hard
> to figure out where they come from, since typically deep in the call chain
> of log tree walking we return errors and propagate them up to the caller of
> the main log tree walk function, which then aborts the transaction or turns
> the filesystem into error state (btrfs_handle_fs_error()). This means any
> stack trace and message provided by a transaction abort or turning fs into
> error state, doesn't provide information where exactly in the deep call
> chain the error comes from.
> 
> These changes mostly make transacton aborts and btrfs_handle_fs_error()
> calls where errors happen, so that we get much more useful information
> which sometimes is enough to understand issues. The rest are just some
> cleanups.

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

Although I believe we may further enhance the output by dumping the log 
tree when replay_one_buffer() function failed.

Especially considering we don't have a simple way to dump the log tree 
for a subvolume (always needs to go through the log tree root, then into 
the log tree of a subvolume).

Thanks,
Qu


> 
> Filipe Manana (10):
>    btrfs: error on missing block group when unaccounting log tree extent buffers
>    btrfs: abort transaction on specific error places when walking log tree
>    btrfs: abort transaction in the process_one_buffer() log tree walk callback
>    btrfs: use local variable for the transaction handle in replay_one_buffer()
>    btrfs: return real error from read_alloc_one_name() in drop_one_dir_item()
>    btrfs: abort transaction where errors happen during log tree replay
>    btrfs: exit early when replaying hole file extent item from a log tree
>    btrfs: process inline extent earlier in replay_one_extent()
>    btrfs: use local key variable to pass arguments in replay_one_extent()
>    btrfs: collapse unaccount_log_buffer() into clean_log_buffer()
> 
>   fs/btrfs/tree-log.c | 659 +++++++++++++++++++++++++++-----------------
>   1 file changed, 401 insertions(+), 258 deletions(-)
> 

Re: [PATCH 00/10] btrfs: improve error reporting for log tree replay

[Filipe Manana]

On Tue, Jul 22, 2025 at 1:16 AM Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> 在 2025/7/22 02:46, fdmanana@kernel.org 写道:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Most errors that happen during log replay or destroying a log tree are hard
> > to figure out where they come from, since typically deep in the call chain
> > of log tree walking we return errors and propagate them up to the caller of
> > the main log tree walk function, which then aborts the transaction or turns
> > the filesystem into error state (btrfs_handle_fs_error()). This means any
> > stack trace and message provided by a transaction abort or turning fs into
> > error state, doesn't provide information where exactly in the deep call
> > chain the error comes from.
> >
> > These changes mostly make transacton aborts and btrfs_handle_fs_error()
> > calls where errors happen, so that we get much more useful information
> > which sometimes is enough to understand issues. The rest are just some
> > cleanups.
>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
>
> Although I believe we may further enhance the output by dumping the log
> tree when replay_one_buffer() function failed.
>
> Especially considering we don't have a simple way to dump the log tree
> for a subvolume (always needs to go through the log tree root, then into
> the log tree of a subvolume).

Dumping the log tree alone is not as useful as you might think.

Unless there's an actual IO error due to some extent buffer not
getting persisted, problems during log replay happen because:

1) There are items we didn't log - you can only figure that out by
checking the subvolume tree as well;

2) There are items we logged but should have been removed from a log
tree after events such as unlinks and renames for example - again,
that can figured out only by checking the subvolume tree as well;

So most, if not all problems during log replay, that I remember ever
fixing, and there were lots of them, boiled down to that.
And most times we also need to have an idea of what file/dir
operations happened besides checking the log and subvolume trees.

Also dumping an entire log tree is too much, as it can reach 3 levels
(root at level 2) during the lifetime of a transaction.

There are other things I want to do on top of these changes, but
dumping an entire log tree is not of them.


>
> Thanks,
> Qu
>
>
> >
> > Filipe Manana (10):
> >    btrfs: error on missing block group when unaccounting log tree extent buffers
> >    btrfs: abort transaction on specific error places when walking log tree
> >    btrfs: abort transaction in the process_one_buffer() log tree walk callback
> >    btrfs: use local variable for the transaction handle in replay_one_buffer()
> >    btrfs: return real error from read_alloc_one_name() in drop_one_dir_item()
> >    btrfs: abort transaction where errors happen during log tree replay
> >    btrfs: exit early when replaying hole file extent item from a log tree
> >    btrfs: process inline extent earlier in replay_one_extent()
> >    btrfs: use local key variable to pass arguments in replay_one_extent()
> >    btrfs: collapse unaccount_log_buffer() into clean_log_buffer()
> >
> >   fs/btrfs/tree-log.c | 659 +++++++++++++++++++++++++++-----------------
> >   1 file changed, 401 insertions(+), 258 deletions(-)
> >
>
>

Re: [PATCH 00/10] btrfs: improve error reporting for log tree replay

[Qu Wenruo]

在 2025/7/22 19:34, Filipe Manana 写道:
> On Tue, Jul 22, 2025 at 1:16 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>>
>>
>> 在 2025/7/22 02:46, fdmanana@kernel.org 写道:
>>> From: Filipe Manana <fdmanana@suse.com>
>>>
>>> Most errors that happen during log replay or destroying a log tree are hard
>>> to figure out where they come from, since typically deep in the call chain
>>> of log tree walking we return errors and propagate them up to the caller of
>>> the main log tree walk function, which then aborts the transaction or turns
>>> the filesystem into error state (btrfs_handle_fs_error()). This means any
>>> stack trace and message provided by a transaction abort or turning fs into
>>> error state, doesn't provide information where exactly in the deep call
>>> chain the error comes from.
>>>
>>> These changes mostly make transacton aborts and btrfs_handle_fs_error()
>>> calls where errors happen, so that we get much more useful information
>>> which sometimes is enough to understand issues. The rest are just some
>>> cleanups.
>>
>> Reviewed-by: Qu Wenruo <wqu@suse.com>
>>
>> Although I believe we may further enhance the output by dumping the log
>> tree when replay_one_buffer() function failed.
>>
>> Especially considering we don't have a simple way to dump the log tree
>> for a subvolume (always needs to go through the log tree root, then into
>> the log tree of a subvolume).
> 
> Dumping the log tree alone is not as useful as you might think.
> 
> Unless there's an actual IO error due to some extent buffer not
> getting persisted, problems during log replay happen because:
> 
> 1) There are items we didn't log - you can only figure that out by
> checking the subvolume tree as well;
> 
> 2) There are items we logged but should have been removed from a log
> tree after events such as unlinks and renames for example - again,
> that can figured out only by checking the subvolume tree as well;
> 
> So most, if not all problems during log replay, that I remember ever
> fixing, and there were lots of them, boiled down to that.
> And most times we also need to have an idea of what file/dir
> operations happened besides checking the log and subvolume trees.
> 
> Also dumping an entire log tree is too much, as it can reach 3 levels
> (root at level 2) during the lifetime of a transaction.
> 
> There are other things I want to do on top of these changes, but
> dumping an entire log tree is not of them.

My bad, since we're at replay_one_buffer() the more sane one is to dump 
that log tree leaf.

And the subvolume leaf is also at reach, e.g. dump path->nodes[0] if 
there is an error and we have path->nodes[0] populated inside various 
replay_*() functions.

Thanks,
Qu

> 
> 
>>
>> Thanks,
>> Qu
>>
>>
>>>
>>> Filipe Manana (10):
>>>     btrfs: error on missing block group when unaccounting log tree extent buffers
>>>     btrfs: abort transaction on specific error places when walking log tree
>>>     btrfs: abort transaction in the process_one_buffer() log tree walk callback
>>>     btrfs: use local variable for the transaction handle in replay_one_buffer()
>>>     btrfs: return real error from read_alloc_one_name() in drop_one_dir_item()
>>>     btrfs: abort transaction where errors happen during log tree replay
>>>     btrfs: exit early when replaying hole file extent item from a log tree
>>>     btrfs: process inline extent earlier in replay_one_extent()
>>>     btrfs: use local key variable to pass arguments in replay_one_extent()
>>>     btrfs: collapse unaccount_log_buffer() into clean_log_buffer()
>>>
>>>    fs/btrfs/tree-log.c | 659 +++++++++++++++++++++++++++-----------------
>>>    1 file changed, 401 insertions(+), 258 deletions(-)
>>>
>>
>>
> 

Re: [PATCH 00/10] btrfs: improve error reporting for log tree replay

[Filipe Manana]

On Mon, Jul 21, 2025 at 9:22 PM Boris Burkov <boris@bur.io> wrote:
>
> On Mon, Jul 21, 2025 at 06:16:27PM +0100, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Most errors that happen during log replay or destroying a log tree are hard
> > to figure out where they come from, since typically deep in the call chain
> > of log tree walking we return errors and propagate them up to the caller of
> > the main log tree walk function, which then aborts the transaction or turns
> > the filesystem into error state (btrfs_handle_fs_error()). This means any
> > stack trace and message provided by a transaction abort or turning fs into
> > error state, doesn't provide information where exactly in the deep call
> > chain the error comes from.
> >
> > These changes mostly make transacton aborts and btrfs_handle_fs_error()
> > calls where errors happen, so that we get much more useful information
> > which sometimes is enough to understand issues. The rest are just some
> > cleanups.
>
> Reviewed-by: Boris Burkov <boris@bur.io>

Don't forget to hit "reply all" so that the list gets cc'ed :)

>
> >
> > Filipe Manana (10):
> >   btrfs: error on missing block group when unaccounting log tree extent buffers
> >   btrfs: abort transaction on specific error places when walking log tree
> >   btrfs: abort transaction in the process_one_buffer() log tree walk callback
> >   btrfs: use local variable for the transaction handle in replay_one_buffer()
> >   btrfs: return real error from read_alloc_one_name() in drop_one_dir_item()
> >   btrfs: abort transaction where errors happen during log tree replay
> >   btrfs: exit early when replaying hole file extent item from a log tree
> >   btrfs: process inline extent earlier in replay_one_extent()
> >   btrfs: use local key variable to pass arguments in replay_one_extent()
> >   btrfs: collapse unaccount_log_buffer() into clean_log_buffer()
> >
> >  fs/btrfs/tree-log.c | 659 +++++++++++++++++++++++++++-----------------
> >  1 file changed, 401 insertions(+), 258 deletions(-)
> >
> > --
> > 2.47.2
> >