[PATCH 0/6] btrfs: some speedup for NOCOW write path and cleanups

[PATCH 0/6] btrfs: some speedup for NOCOW write path and cleanups

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

These are some cleanups in the NOCOW write path and making the check for
the existence of checksums in a range more efficient. More details in the
changelogs.

Filipe Manana (6):
  btrfs: add function comment to btrfs_lookup_csums_list()
  btrfs: remove search_commit parameter from btrfs_lookup_csums_list()
  btrfs: remove use of a temporary list at btrfs_lookup_csums_list()
  btrfs: simplify error path for btrfs_lookup_csums_list()
  btrfs: make NOCOW checks for existence of checksums in a range more efficient
  btrfs: open code csum_exist_in_range()

 fs/btrfs/file-item.c  | 57 +++++++++++++++++++++++++++----------------
 fs/btrfs/file-item.h  |  3 +--
 fs/btrfs/inode.c      | 33 ++++++-------------------
 fs/btrfs/relocation.c |  4 +--
 fs/btrfs/tree-log.c   | 15 +++++++-----
 5 files changed, 55 insertions(+), 57 deletions(-)

-- 
2.43.0

[PATCH 1/6] btrfs: add function comment to btrfs_lookup_csums_list()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Add a function comment to btrfs_lookup_csums_list() to document it.
With another upcoming change its parameter list and return value will be
less obvious. So add the documentation now so that it can be updated where
needed later.

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

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index e58fb5347e65..909438540119 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -450,6 +450,19 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 	return ret;
 }
 
+/*
+ * Search for checksums for a given logical range.
+ *
+ * @root:		The root where to look for checksums.
+ * @start:		Logical address of target checksum range.
+ * @end:		End offset (inclusive) of the target checksum range.
+ * @list:		List for adding each checksum that was found.
+ * @search_commit:	Indicate if the commit root of the @root should be used
+ *			for the search.
+ * @nowait:		Indicate if the search must be non-blocking or not.
+ *
+ * Return < 0 on error and 0 on success.
+ */
 int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 			    struct list_head *list, int search_commit,
 			    bool nowait)
-- 
2.43.0

[PATCH 2/6] btrfs: remove search_commit parameter from btrfs_lookup_csums_list()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

All the callers of btrfs_lookup_csums_list() pass a value of 0 as the
"search_commit" parameter. So remove it and make the function behave as
to always search from the regular root.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/file-item.c  | 10 +---------
 fs/btrfs/file-item.h  |  3 +--
 fs/btrfs/inode.c      |  2 +-
 fs/btrfs/relocation.c |  2 +-
 fs/btrfs/tree-log.c   |  6 +++---
 5 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 909438540119..0712a0aa2dd0 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -457,15 +457,12 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
  * @start:		Logical address of target checksum range.
  * @end:		End offset (inclusive) of the target checksum range.
  * @list:		List for adding each checksum that was found.
- * @search_commit:	Indicate if the commit root of the @root should be used
- *			for the search.
  * @nowait:		Indicate if the search must be non-blocking or not.
  *
  * Return < 0 on error and 0 on success.
  */
 int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
-			    struct list_head *list, int search_commit,
-			    bool nowait)
+			    struct list_head *list, bool nowait)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_key key;
@@ -484,11 +481,6 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 		return -ENOMEM;
 
 	path->nowait = nowait;
-	if (search_commit) {
-		path->skip_locking = 1;
-		path->reada = READA_FORWARD;
-		path->search_commit_root = 1;
-	}
 
 	key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
 	key.offset = start;
diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
index 15c05cc0fce6..557dc43d7142 100644
--- a/fs/btrfs/file-item.h
+++ b/fs/btrfs/file-item.h
@@ -68,8 +68,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 			     struct list_head *list, int search_commit,
 			     bool nowait);
 int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
-			    struct list_head *list, int search_commit,
-			    bool nowait);
+			    struct list_head *list, bool nowait);
 int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
 			      u64 start, u64 end, u8 *csum_buf,
 			      unsigned long *csum_bitmap);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2dae4e975e80..4e67470d847a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1746,7 +1746,7 @@ static noinline int csum_exist_in_range(struct btrfs_fs_info *fs_info,
 	LIST_HEAD(list);
 
 	ret = btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
-				      &list, 0, nowait);
+				      &list, nowait);
 	if (ret == 0 && list_empty(&list))
 		return 0;
 
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 5b19b41f64a2..4e60364b5289 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4391,7 +4391,7 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
 
 	ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 				      disk_bytenr + ordered->num_bytes - 1,
-				      &list, 0, false);
+				      &list, false);
 	if (ret)
 		return ret;
 
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 4a4fca841510..e9ad2971fc7c 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -797,7 +797,7 @@ 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, 0, false);
+						&ordered_sums, false);
 			if (ret)
 				goto out;
 			/*
@@ -4460,7 +4460,7 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
 		disk_bytenr += extent_offset;
 		ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 					      disk_bytenr + extent_num_bytes - 1,
-					      &ordered_sums, 0, false);
+					      &ordered_sums, false);
 		if (ret)
 			goto out;
 
@@ -4655,7 +4655,7 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
 	csum_root = btrfs_csum_root(trans->fs_info, em->block_start);
 	ret = btrfs_lookup_csums_list(csum_root, em->block_start + csum_offset,
 				      em->block_start + csum_offset +
-				      csum_len - 1, &ordered_sums, 0, false);
+				      csum_len - 1, &ordered_sums, false);
 	if (ret)
 		return ret;
 
-- 
2.43.0

[PATCH 3/6] btrfs: remove use of a temporary list at btrfs_lookup_csums_list()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There's no need to use a temporary list to add the checksums, we can just
add them to input list and then on error delete and free any checksums
that were added. So simplify and remove the temporary list.

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

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 0712a0aa2dd0..c2799325706f 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -470,7 +470,6 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 	struct extent_buffer *leaf;
 	struct btrfs_ordered_sum *sums;
 	struct btrfs_csum_item *item;
-	LIST_HEAD(tmplist);
 	int ret;
 
 	ASSERT(IS_ALIGNED(start, fs_info->sectorsize) &&
@@ -572,18 +571,17 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 					   bytes_to_csum_size(fs_info, size));
 
 			start += size;
-			list_add_tail(&sums->list, &tmplist);
+			list_add_tail(&sums->list, list);
 		}
 		path->slots[0]++;
 	}
 	ret = 0;
 fail:
-	while (ret < 0 && !list_empty(&tmplist)) {
-		sums = list_entry(tmplist.next, struct btrfs_ordered_sum, list);
+	while (ret < 0 && !list_empty(list)) {
+		sums = list_entry(list->next, struct btrfs_ordered_sum, list);
 		list_del(&sums->list);
 		kfree(sums);
 	}
-	list_splice_tail(&tmplist, list);
 
 	btrfs_free_path(path);
 	return ret;
-- 
2.43.0

[PATCH 4/6] btrfs: simplify error path for btrfs_lookup_csums_list()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

In the error path we have this while loop that keeps iterating over the
csums of the list and then delete them from the list and free them,
testing for an error (ret < 0) and list emptyness as the conditions of
the while loop.

Simplify this by using list_for_each_entry_safe() so there's no need to
delete elements from the list and need to test the error condition on
each iteration.

Also rename the 'fail' label to 'out' since the label is not exclusive
to a failure path, as we also end up there when the function succeeds,
and it's also a more common label name.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/file-item.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index c2799325706f..231abcc87f63 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -487,7 +487,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
 	if (ret < 0)
-		goto fail;
+		goto out;
 	if (ret > 0 && path->slots[0] > 0) {
 		leaf = path->nodes[0];
 		btrfs_item_key_to_cpu(leaf, &key, path->slots[0] - 1);
@@ -522,7 +522,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 		if (path->slots[0] >= btrfs_header_nritems(leaf)) {
 			ret = btrfs_next_leaf(root, path);
 			if (ret < 0)
-				goto fail;
+				goto out;
 			if (ret > 0)
 				break;
 			leaf = path->nodes[0];
@@ -557,7 +557,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 				       GFP_NOFS);
 			if (!sums) {
 				ret = -ENOMEM;
-				goto fail;
+				goto out;
 			}
 
 			sums->logical = start;
@@ -576,11 +576,12 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 		path->slots[0]++;
 	}
 	ret = 0;
-fail:
-	while (ret < 0 && !list_empty(list)) {
-		sums = list_entry(list->next, struct btrfs_ordered_sum, list);
-		list_del(&sums->list);
-		kfree(sums);
+out:
+	if (ret < 0) {
+		struct btrfs_ordered_sum *tmp_sums;
+
+		list_for_each_entry_safe(sums, tmp_sums, list, list)
+			kfree(sums);
 	}
 
 	btrfs_free_path(path);
-- 
2.43.0

[PATCH 5/6] btrfs: make NOCOW checks for existence of checksums in a range more efficient

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Before deciding if we can do a NOCOW write into a range, one of the things
we have to do is check if there are checksum items for that range. We do
that through the btrfs_lookup_csums_list() function, which searches for
checksums and adds them to a list supplied by the caller.

But all we need is to check if there is any checksum, we don't need to
look for all of them and collect them into a list, which requires more
search time in the checksums tree, allocating memory for checksums items
to add to the list, copy checksums from a leaf into those list items,
then free that memory, etc. This is all unnecessary overhead, wasting
mostly CPU time, and perhaps some occasional IO if we need to read from
disk any extent buffers.

So change btrfs_lookup_csums_list() to allow to return immediately in
case it finds any checksum, without the need to add it to a list and read
it from a leaf. This is accomplised by allowing a NULL list parameter and
making the function return 1 if it found any checksum, 0 if it didn't
found any, and a negative value in case of an error.

The following test with fio was used to measure performance:

  $ cat test.sh
  #!/bin/bash

  DEV=/dev/nullb0
  MNT=/mnt/nullb0

  cat <<EOF > /tmp/fio-job.ini
  [global]
  name=fio-rand-write
  filename=$MNT/fio-rand-write
  rw=randwrite
  bssplit=4k/20:8k/20:16k/20:32k/20:64k/20
  direct=1
  numjobs=16
  fallocate=posix
  time_based
  runtime=300

  [file1]
  size=8G
  ioengine=io_uring
  iodepth=16
  EOF

  umount $MNT &> /dev/null
  mkfs.btrfs -f $DEV
  mount -o ssd $DEV $MNT

  fio /tmp/fio-job.ini
  umount $MNT

The test was run on a release kernel (Debian's default kernel config).

The results before this patch:

  WRITE: bw=139MiB/s (146MB/s), 8204KiB/s-9504KiB/s (8401kB/s-9732kB/s), io=17.0GiB (18.3GB), run=125317-125344msec

The results after this patch:

  WRITE: bw=153MiB/s (160MB/s), 9241KiB/s-10.0MiB/s (9463kB/s-10.5MB/s), io=17.0GiB (18.3GB), run=114054-114071msec

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/file-item.c  | 25 ++++++++++++++++++-------
 fs/btrfs/inode.c      | 18 ++----------------
 fs/btrfs/relocation.c |  2 +-
 fs/btrfs/tree-log.c   |  9 ++++++---
 4 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 231abcc87f63..1ea1ed44fe42 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -457,9 +457,12 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
  * @start:		Logical address of target checksum range.
  * @end:		End offset (inclusive) of the target checksum range.
  * @list:		List for adding each checksum that was found.
+ *			Can be NULL in case the caller only wants to check if
+ *			there any checksums for the range.
  * @nowait:		Indicate if the search must be non-blocking or not.
  *
- * Return < 0 on error and 0 on success.
+ * Return < 0 on error, 0 if no checksums were found, or 1 if checksums were
+ * found.
  */
 int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 			    struct list_head *list, bool nowait)
@@ -471,6 +474,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 	struct btrfs_ordered_sum *sums;
 	struct btrfs_csum_item *item;
 	int ret;
+	bool found_csums = false;
 
 	ASSERT(IS_ALIGNED(start, fs_info->sectorsize) &&
 	       IS_ALIGNED(end + 1, fs_info->sectorsize));
@@ -544,6 +548,10 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 			continue;
 		}
 
+		found_csums = true;
+		if (!list)
+			goto out;
+
 		csum_end = min(csum_end, end + 1);
 		item = btrfs_item_ptr(path->nodes[0], path->slots[0],
 				      struct btrfs_csum_item);
@@ -575,17 +583,20 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
 		}
 		path->slots[0]++;
 	}
-	ret = 0;
 out:
+	btrfs_free_path(path);
 	if (ret < 0) {
-		struct btrfs_ordered_sum *tmp_sums;
+		if (list) {
+			struct btrfs_ordered_sum *tmp_sums;
 
-		list_for_each_entry_safe(sums, tmp_sums, list, list)
-			kfree(sums);
+			list_for_each_entry_safe(sums, tmp_sums, list, list)
+				kfree(sums);
+		}
+
+		return ret;
 	}
 
-	btrfs_free_path(path);
-	return ret;
+	return found_csums ? 1 : 0;
 }
 
 /*
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 4e67470d847a..1d78e07d082b 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1741,23 +1741,9 @@ static noinline int csum_exist_in_range(struct btrfs_fs_info *fs_info,
 					u64 bytenr, u64 num_bytes, bool nowait)
 {
 	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, bytenr);
-	struct btrfs_ordered_sum *sums;
-	int ret;
-	LIST_HEAD(list);
-
-	ret = btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
-				      &list, nowait);
-	if (ret == 0 && list_empty(&list))
-		return 0;
 
-	while (!list_empty(&list)) {
-		sums = list_entry(list.next, struct btrfs_ordered_sum, list);
-		list_del(&sums->list);
-		kfree(sums);
-	}
-	if (ret < 0)
-		return ret;
-	return 1;
+	return btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
+				       NULL, nowait);
 }
 
 static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page,
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 4e60364b5289..5a01aaa164de 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4392,7 +4392,7 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
 	ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 				      disk_bytenr + ordered->num_bytes - 1,
 				      &list, false);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	while (!list_empty(&list)) {
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index e9ad2971fc7c..3c86fcebafc6 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -798,8 +798,9 @@ 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)
+			if (ret < 0)
 				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
@@ -4461,8 +4462,9 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
 		ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 					      disk_bytenr + extent_num_bytes - 1,
 					      &ordered_sums, false);
-		if (ret)
+		if (ret < 0)
 			goto out;
+		ret = 0;
 
 		list_for_each_entry_safe(sums, sums_next, &ordered_sums, list) {
 			if (!ret)
@@ -4656,8 +4658,9 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
 	ret = btrfs_lookup_csums_list(csum_root, em->block_start + csum_offset,
 				      em->block_start + csum_offset +
 				      csum_len - 1, &ordered_sums, false);
-	if (ret)
+	if (ret < 0)
 		return ret;
+	ret = 0;
 
 	while (!list_empty(&ordered_sums)) {
 		struct btrfs_ordered_sum *sums = list_entry(ordered_sums.next,
-- 
2.43.0

[PATCH 6/6] btrfs: open code csum_exist_in_range()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

The csum_exist_in_range() function is now too trivial and is only used in
one place, so open code it in its single caller.

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

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1d78e07d082b..f23511428e74 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1737,15 +1737,6 @@ static noinline int run_delalloc_cow(struct btrfs_inode *inode,
 	return 1;
 }
 
-static noinline int csum_exist_in_range(struct btrfs_fs_info *fs_info,
-					u64 bytenr, u64 num_bytes, bool nowait)
-{
-	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, bytenr);
-
-	return btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
-				       NULL, nowait);
-}
-
 static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page,
 			   const u64 start, const u64 end)
 {
@@ -1860,6 +1851,7 @@ static int can_nocow_file_extent(struct btrfs_path *path,
 	struct extent_buffer *leaf = path->nodes[0];
 	struct btrfs_root *root = inode->root;
 	struct btrfs_file_extent_item *fi;
+	struct btrfs_root *csum_root;
 	u64 extent_end;
 	u8 extent_type;
 	int can_nocow = 0;
@@ -1920,7 +1912,7 @@ static int can_nocow_file_extent(struct btrfs_path *path,
 	if (args->free_path) {
 		/*
 		 * We don't need the path anymore, plus through the
-		 * csum_exist_in_range() call below we will end up allocating
+		 * btrfs_lookup_csums_list() call below we will end up allocating
 		 * another path. So free the path to avoid unnecessary extra
 		 * memory usage.
 		 */
@@ -1941,8 +1933,11 @@ static int can_nocow_file_extent(struct btrfs_path *path,
 	 * Force COW if csums exist in the range. This ensures that csums for a
 	 * given extent are either valid or do not exist.
 	 */
-	ret = csum_exist_in_range(root->fs_info, args->disk_bytenr, args->num_bytes,
-				  nowait);
+
+	csum_root = btrfs_csum_root(root->fs_info, args->disk_bytenr);
+	ret = btrfs_lookup_csums_list(csum_root, args->disk_bytenr,
+				      args->disk_bytenr + args->num_bytes - 1,
+				      NULL, nowait);
 	WARN_ON_ONCE(ret > 0 && is_freespace_inode);
 	if (ret != 0)
 		goto out;
-- 
2.43.0

Re: [PATCH 0/6] btrfs: some speedup for NOCOW write path and cleanups

[David Sterba]

On Fri, Apr 12, 2024 at 04:03:14PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> These are some cleanups in the NOCOW write path and making the check for
> the existence of checksums in a range more efficient. More details in the
> changelogs.
> 
> Filipe Manana (6):
>   btrfs: add function comment to btrfs_lookup_csums_list()
>   btrfs: remove search_commit parameter from btrfs_lookup_csums_list()
>   btrfs: remove use of a temporary list at btrfs_lookup_csums_list()
>   btrfs: simplify error path for btrfs_lookup_csums_list()
>   btrfs: make NOCOW checks for existence of checksums in a range more efficient
>   btrfs: open code csum_exist_in_range()

Reviewed-by: David Sterba <dsterba@suse.com>

Re: [PATCH 3/6] btrfs: remove use of a temporary list at btrfs_lookup_csums_list()

[Qu Wenruo]

在 2024/4/13 00:33, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> There's no need to use a temporary list to add the checksums, we can just
> add them to input list and then on error delete and free any checksums
> that were added. So simplify and remove the temporary list.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Initially I'm not sure if this would bring a behavior change as csum
tree search failure would free any existing csum items, but all the call
sites pass an empty list into the function anyway, so it's completely fine.

Thanks,
Qu
> ---
>   fs/btrfs/file-item.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 0712a0aa2dd0..c2799325706f 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -470,7 +470,6 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   	struct extent_buffer *leaf;
>   	struct btrfs_ordered_sum *sums;
>   	struct btrfs_csum_item *item;
> -	LIST_HEAD(tmplist);
>   	int ret;
>
>   	ASSERT(IS_ALIGNED(start, fs_info->sectorsize) &&
> @@ -572,18 +571,17 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   					   bytes_to_csum_size(fs_info, size));
>
>   			start += size;
> -			list_add_tail(&sums->list, &tmplist);
> +			list_add_tail(&sums->list, list);
>   		}
>   		path->slots[0]++;
>   	}
>   	ret = 0;
>   fail:
> -	while (ret < 0 && !list_empty(&tmplist)) {
> -		sums = list_entry(tmplist.next, struct btrfs_ordered_sum, list);
> +	while (ret < 0 && !list_empty(list)) {
> +		sums = list_entry(list->next, struct btrfs_ordered_sum, list);
>   		list_del(&sums->list);
>   		kfree(sums);
>   	}
> -	list_splice_tail(&tmplist, list);
>
>   	btrfs_free_path(path);
>   	return ret;

Re: [PATCH 2/6] btrfs: remove search_commit parameter from btrfs_lookup_csums_list()

[Qu Wenruo]

在 2024/4/13 00:33, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> All the callers of btrfs_lookup_csums_list() pass a value of 0 as the
> "search_commit" parameter. So remove it and make the function behave as
> to always search from the regular root.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

The last user of search_commit was removed in commit 5dc96f8d5de9
("btrfs: scrub: remove scrub_parity structure"), as we migrate to a
bitmap based solution.

Thanks,
Qu
> ---
>   fs/btrfs/file-item.c  | 10 +---------
>   fs/btrfs/file-item.h  |  3 +--
>   fs/btrfs/inode.c      |  2 +-
>   fs/btrfs/relocation.c |  2 +-
>   fs/btrfs/tree-log.c   |  6 +++---
>   5 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 909438540119..0712a0aa2dd0 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -457,15 +457,12 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>    * @start:		Logical address of target checksum range.
>    * @end:		End offset (inclusive) of the target checksum range.
>    * @list:		List for adding each checksum that was found.
> - * @search_commit:	Indicate if the commit root of the @root should be used
> - *			for the search.
>    * @nowait:		Indicate if the search must be non-blocking or not.
>    *
>    * Return < 0 on error and 0 on success.
>    */
>   int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
> -			    struct list_head *list, int search_commit,
> -			    bool nowait)
> +			    struct list_head *list, bool nowait)
>   {
>   	struct btrfs_fs_info *fs_info = root->fs_info;
>   	struct btrfs_key key;
> @@ -484,11 +481,6 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   		return -ENOMEM;
>
>   	path->nowait = nowait;
> -	if (search_commit) {
> -		path->skip_locking = 1;
> -		path->reada = READA_FORWARD;
> -		path->search_commit_root = 1;
> -	}
>
>   	key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
>   	key.offset = start;
> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> index 15c05cc0fce6..557dc43d7142 100644
> --- a/fs/btrfs/file-item.h
> +++ b/fs/btrfs/file-item.h
> @@ -68,8 +68,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>   			     struct list_head *list, int search_commit,
>   			     bool nowait);
>   int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
> -			    struct list_head *list, int search_commit,
> -			    bool nowait);
> +			    struct list_head *list, bool nowait);
>   int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
>   			      u64 start, u64 end, u8 *csum_buf,
>   			      unsigned long *csum_bitmap);
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 2dae4e975e80..4e67470d847a 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1746,7 +1746,7 @@ static noinline int csum_exist_in_range(struct btrfs_fs_info *fs_info,
>   	LIST_HEAD(list);
>
>   	ret = btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
> -				      &list, 0, nowait);
> +				      &list, nowait);
>   	if (ret == 0 && list_empty(&list))
>   		return 0;
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 5b19b41f64a2..4e60364b5289 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4391,7 +4391,7 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
>
>   	ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
>   				      disk_bytenr + ordered->num_bytes - 1,
> -				      &list, 0, false);
> +				      &list, false);
>   	if (ret)
>   		return ret;
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 4a4fca841510..e9ad2971fc7c 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -797,7 +797,7 @@ 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, 0, false);
> +						&ordered_sums, false);
>   			if (ret)
>   				goto out;
>   			/*
> @@ -4460,7 +4460,7 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
>   		disk_bytenr += extent_offset;
>   		ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
>   					      disk_bytenr + extent_num_bytes - 1,
> -					      &ordered_sums, 0, false);
> +					      &ordered_sums, false);
>   		if (ret)
>   			goto out;
>
> @@ -4655,7 +4655,7 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
>   	csum_root = btrfs_csum_root(trans->fs_info, em->block_start);
>   	ret = btrfs_lookup_csums_list(csum_root, em->block_start + csum_offset,
>   				      em->block_start + csum_offset +
> -				      csum_len - 1, &ordered_sums, 0, false);
> +				      csum_len - 1, &ordered_sums, false);
>   	if (ret)
>   		return ret;
>

Re: [PATCH 1/6] btrfs: add function comment to btrfs_lookup_csums_list()

[Qu Wenruo]

在 2024/4/13 00:33, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> Add a function comment to btrfs_lookup_csums_list() to document it.
> With another upcoming change its parameter list and return value will be
> less obvious. So add the documentation now so that it can be updated where
> needed later.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/file-item.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index e58fb5347e65..909438540119 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -450,6 +450,19 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>   	return ret;
>   }
>
> +/*
> + * Search for checksums for a given logical range.
> + *
> + * @root:		The root where to look for checksums.
> + * @start:		Logical address of target checksum range.
> + * @end:		End offset (inclusive) of the target checksum range.
> + * @list:		List for adding each checksum that was found.
> + * @search_commit:	Indicate if the commit root of the @root should be used
> + *			for the search.
> + * @nowait:		Indicate if the search must be non-blocking or not.
> + *
> + * Return < 0 on error and 0 on success.
> + */
>   int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   			    struct list_head *list, int search_commit,
>   			    bool nowait)

Re: [PATCH 4/6] btrfs: simplify error path for btrfs_lookup_csums_list()

[Qu Wenruo]

在 2024/4/13 00:33, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> In the error path we have this while loop that keeps iterating over the
> csums of the list and then delete them from the list and free them,
> testing for an error (ret < 0) and list emptyness as the conditions of
> the while loop.
>
> Simplify this by using list_for_each_entry_safe() so there's no need to
> delete elements from the list and need to test the error condition on
> each iteration.
>
> Also rename the 'fail' label to 'out' since the label is not exclusive
> to a failure path, as we also end up there when the function succeeds,
> and it's also a more common label name.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/file-item.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index c2799325706f..231abcc87f63 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -487,7 +487,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>
>   	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>   	if (ret < 0)
> -		goto fail;
> +		goto out;
>   	if (ret > 0 && path->slots[0] > 0) {
>   		leaf = path->nodes[0];
>   		btrfs_item_key_to_cpu(leaf, &key, path->slots[0] - 1);
> @@ -522,7 +522,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   		if (path->slots[0] >= btrfs_header_nritems(leaf)) {
>   			ret = btrfs_next_leaf(root, path);
>   			if (ret < 0)
> -				goto fail;
> +				goto out;
>   			if (ret > 0)
>   				break;
>   			leaf = path->nodes[0];
> @@ -557,7 +557,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   				       GFP_NOFS);
>   			if (!sums) {
>   				ret = -ENOMEM;
> -				goto fail;
> +				goto out;
>   			}
>
>   			sums->logical = start;
> @@ -576,11 +576,12 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   		path->slots[0]++;
>   	}
>   	ret = 0;
> -fail:
> -	while (ret < 0 && !list_empty(list)) {
> -		sums = list_entry(list->next, struct btrfs_ordered_sum, list);
> -		list_del(&sums->list);
> -		kfree(sums);
> +out:
> +	if (ret < 0) {
> +		struct btrfs_ordered_sum *tmp_sums;
> +
> +		list_for_each_entry_safe(sums, tmp_sums, list, list)
> +			kfree(sums);
>   	}
>
>   	btrfs_free_path(path);

Re: [PATCH 5/6] btrfs: make NOCOW checks for existence of checksums in a range more efficient

[Qu Wenruo]

在 2024/4/13 00:33, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> Before deciding if we can do a NOCOW write into a range, one of the things
> we have to do is check if there are checksum items for that range. We do
> that through the btrfs_lookup_csums_list() function, which searches for
> checksums and adds them to a list supplied by the caller.
>
> But all we need is to check if there is any checksum, we don't need to
> look for all of them and collect them into a list, which requires more
> search time in the checksums tree, allocating memory for checksums items
> to add to the list, copy checksums from a leaf into those list items,
> then free that memory, etc. This is all unnecessary overhead, wasting
> mostly CPU time, and perhaps some occasional IO if we need to read from
> disk any extent buffers.
>
> So change btrfs_lookup_csums_list() to allow to return immediately in
> case it finds any checksum, without the need to add it to a list and read
> it from a leaf. This is accomplised by allowing a NULL list parameter and
> making the function return 1 if it found any checksum, 0 if it didn't
> found any, and a negative value in case of an error.
>
> The following test with fio was used to measure performance:
>
>    $ cat test.sh
>    #!/bin/bash
>
>    DEV=/dev/nullb0
>    MNT=/mnt/nullb0
>
>    cat <<EOF > /tmp/fio-job.ini
>    [global]
>    name=fio-rand-write
>    filename=$MNT/fio-rand-write
>    rw=randwrite
>    bssplit=4k/20:8k/20:16k/20:32k/20:64k/20
>    direct=1
>    numjobs=16
>    fallocate=posix
>    time_based
>    runtime=300
>
>    [file1]
>    size=8G
>    ioengine=io_uring
>    iodepth=16
>    EOF
>
>    umount $MNT &> /dev/null
>    mkfs.btrfs -f $DEV
>    mount -o ssd $DEV $MNT
>
>    fio /tmp/fio-job.ini
>    umount $MNT
>
> The test was run on a release kernel (Debian's default kernel config).
>
> The results before this patch:
>
>    WRITE: bw=139MiB/s (146MB/s), 8204KiB/s-9504KiB/s (8401kB/s-9732kB/s), io=17.0GiB (18.3GB), run=125317-125344msec
>
> The results after this patch:
>
>    WRITE: bw=153MiB/s (160MB/s), 9241KiB/s-10.0MiB/s (9463kB/s-10.5MB/s), io=17.0GiB (18.3GB), run=114054-114071msec
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/file-item.c  | 25 ++++++++++++++++++-------
>   fs/btrfs/inode.c      | 18 ++----------------
>   fs/btrfs/relocation.c |  2 +-
>   fs/btrfs/tree-log.c   |  9 ++++++---
>   4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 231abcc87f63..1ea1ed44fe42 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -457,9 +457,12 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>    * @start:		Logical address of target checksum range.
>    * @end:		End offset (inclusive) of the target checksum range.
>    * @list:		List for adding each checksum that was found.
> + *			Can be NULL in case the caller only wants to check if
> + *			there any checksums for the range.
>    * @nowait:		Indicate if the search must be non-blocking or not.
>    *
> - * Return < 0 on error and 0 on success.
> + * Return < 0 on error, 0 if no checksums were found, or 1 if checksums were
> + * found.
>    */
>   int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   			    struct list_head *list, bool nowait)
> @@ -471,6 +474,7 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   	struct btrfs_ordered_sum *sums;
>   	struct btrfs_csum_item *item;
>   	int ret;
> +	bool found_csums = false;
>
>   	ASSERT(IS_ALIGNED(start, fs_info->sectorsize) &&
>   	       IS_ALIGNED(end + 1, fs_info->sectorsize));
> @@ -544,6 +548,10 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   			continue;
>   		}
>
> +		found_csums = true;
> +		if (!list)
> +			goto out;
> +
>   		csum_end = min(csum_end, end + 1);
>   		item = btrfs_item_ptr(path->nodes[0], path->slots[0],
>   				      struct btrfs_csum_item);
> @@ -575,17 +583,20 @@ int btrfs_lookup_csums_list(struct btrfs_root *root, u64 start, u64 end,
>   		}
>   		path->slots[0]++;
>   	}
> -	ret = 0;
>   out:
> +	btrfs_free_path(path);
>   	if (ret < 0) {
> -		struct btrfs_ordered_sum *tmp_sums;
> +		if (list) {
> +			struct btrfs_ordered_sum *tmp_sums;
>
> -		list_for_each_entry_safe(sums, tmp_sums, list, list)
> -			kfree(sums);
> +			list_for_each_entry_safe(sums, tmp_sums, list, list)
> +				kfree(sums);
> +		}
> +
> +		return ret;
>   	}
>
> -	btrfs_free_path(path);
> -	return ret;
> +	return found_csums ? 1 : 0;
>   }
>
>   /*
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 4e67470d847a..1d78e07d082b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1741,23 +1741,9 @@ static noinline int csum_exist_in_range(struct btrfs_fs_info *fs_info,
>   					u64 bytenr, u64 num_bytes, bool nowait)
>   {
>   	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, bytenr);
> -	struct btrfs_ordered_sum *sums;
> -	int ret;
> -	LIST_HEAD(list);
> -
> -	ret = btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
> -				      &list, nowait);
> -	if (ret == 0 && list_empty(&list))
> -		return 0;
>
> -	while (!list_empty(&list)) {
> -		sums = list_entry(list.next, struct btrfs_ordered_sum, list);
> -		list_del(&sums->list);
> -		kfree(sums);
> -	}
> -	if (ret < 0)
> -		return ret;
> -	return 1;
> +	return btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
> +				       NULL, nowait);
>   }
>
>   static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page,
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 4e60364b5289..5a01aaa164de 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4392,7 +4392,7 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
>   	ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
>   				      disk_bytenr + ordered->num_bytes - 1,
>   				      &list, false);
> -	if (ret)
> +	if (ret < 0)
>   		return ret;
>
>   	while (!list_empty(&list)) {
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index e9ad2971fc7c..3c86fcebafc6 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -798,8 +798,9 @@ 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)
> +			if (ret < 0)
>   				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
> @@ -4461,8 +4462,9 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
>   		ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
>   					      disk_bytenr + extent_num_bytes - 1,
>   					      &ordered_sums, false);
> -		if (ret)
> +		if (ret < 0)
>   			goto out;
> +		ret = 0;
>
>   		list_for_each_entry_safe(sums, sums_next, &ordered_sums, list) {
>   			if (!ret)
> @@ -4656,8 +4658,9 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
>   	ret = btrfs_lookup_csums_list(csum_root, em->block_start + csum_offset,
>   				      em->block_start + csum_offset +
>   				      csum_len - 1, &ordered_sums, false);
> -	if (ret)
> +	if (ret < 0)
>   		return ret;
> +	ret = 0;
>
>   	while (!list_empty(&ordered_sums)) {
>   		struct btrfs_ordered_sum *sums = list_entry(ordered_sums.next,

Re: [PATCH 6/6] btrfs: open code csum_exist_in_range()

[Qu Wenruo]

在 2024/4/13 00:33, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> The csum_exist_in_range() function is now too trivial and is only used in
> one place, so open code it in its single caller.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/inode.c | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 1d78e07d082b..f23511428e74 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1737,15 +1737,6 @@ static noinline int run_delalloc_cow(struct btrfs_inode *inode,
>   	return 1;
>   }
>
> -static noinline int csum_exist_in_range(struct btrfs_fs_info *fs_info,
> -					u64 bytenr, u64 num_bytes, bool nowait)
> -{
> -	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, bytenr);
> -
> -	return btrfs_lookup_csums_list(csum_root, bytenr, bytenr + num_bytes - 1,
> -				       NULL, nowait);
> -}
> -
>   static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page,
>   			   const u64 start, const u64 end)
>   {
> @@ -1860,6 +1851,7 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>   	struct extent_buffer *leaf = path->nodes[0];
>   	struct btrfs_root *root = inode->root;
>   	struct btrfs_file_extent_item *fi;
> +	struct btrfs_root *csum_root;
>   	u64 extent_end;
>   	u8 extent_type;
>   	int can_nocow = 0;
> @@ -1920,7 +1912,7 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>   	if (args->free_path) {
>   		/*
>   		 * We don't need the path anymore, plus through the
> -		 * csum_exist_in_range() call below we will end up allocating
> +		 * btrfs_lookup_csums_list() call below we will end up allocating
>   		 * another path. So free the path to avoid unnecessary extra
>   		 * memory usage.
>   		 */
> @@ -1941,8 +1933,11 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>   	 * Force COW if csums exist in the range. This ensures that csums for a
>   	 * given extent are either valid or do not exist.
>   	 */
> -	ret = csum_exist_in_range(root->fs_info, args->disk_bytenr, args->num_bytes,
> -				  nowait);
> +
> +	csum_root = btrfs_csum_root(root->fs_info, args->disk_bytenr);
> +	ret = btrfs_lookup_csums_list(csum_root, args->disk_bytenr,
> +				      args->disk_bytenr + args->num_bytes - 1,
> +				      NULL, nowait);
>   	WARN_ON_ONCE(ret > 0 && is_freespace_inode);
>   	if (ret != 0)
>   		goto out;