diff mbox series

[v2,4/6] btrfs: refactor memcpy_extent_buffer()

Message ID 825deca8e1421418b18d74d43088c81974f9cf69.1689143655.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: preparation patches for the incoming metadata folio conversion | expand

Commit Message

Qu Wenruo July 12, 2023, 6:37 a.m. UTC
[BACKGROUND]
Currently memcpy_extent_buffer() goes a loop where it would stop at
any page boundary inside [dst_offset, dst_offset + len) or [src_offset,
src_offset + len).

This is mostly allowing us to go copy_pages(), but if we're going folio
we will need to handle multi-page (the old behavior) or single folio
(the new optimization).

The current code would be a burden for future changes.

[ENHANCEMENT]
Instead of sticking with copy_pages(), here we utilize
write_extent_buffer() to handle writing into the dst range.

Now we only need to handle the page boundaries inside the source range,
making later switch to folio much easier.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_io.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

Comments

David Sterba July 13, 2023, 11:51 a.m. UTC | #1
For refactoring patches, please try to describe what exactly or how is
being refactored. In this case it's the main loop, so I've updated the
subjects in the patches.

On Wed, Jul 12, 2023 at 02:37:44PM +0800, Qu Wenruo wrote:
> [BACKGROUND]
> Currently memcpy_extent_buffer() goes a loop where it would stop at
> any page boundary inside [dst_offset, dst_offset + len) or [src_offset,
> src_offset + len).
> 
> This is mostly allowing us to go copy_pages(), but if we're going folio
> we will need to handle multi-page (the old behavior) or single folio
> (the new optimization).
> 
> The current code would be a burden for future changes.
> 
> [ENHANCEMENT]
> Instead of sticking with copy_pages(), here we utilize
> write_extent_buffer() to handle writing into the dst range.
> 
> Now we only need to handle the page boundaries inside the source range,
> making later switch to folio much easier.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/extent_io.c | 36 +++++++++++++-----------------------
>  1 file changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 4e252fd7b78a..4db90ede8219 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4183,6 +4183,8 @@ void write_extent_buffer(const struct extent_buffer *eb, const void *srcv,
>  	struct page *page;
>  	char *kaddr;
>  	char *src = (char *)srcv;
> +	/* For unmapped (dummy) ebs, no need to check their uptodate status. */
> +	bool check_uptodate = !test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags);

const bool

>  	unsigned long i = get_eb_page_index(start);
>  
>  	WARN_ON(test_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags));
> @@ -4194,7 +4196,8 @@ void write_extent_buffer(const struct extent_buffer *eb, const void *srcv,
>  
>  	while (len > 0) {
>  		page = eb->pages[i];
> -		assert_eb_page_uptodate(eb, page);
> +		if (check_uptodate)
> +			assert_eb_page_uptodate(eb, page);
>  
>  		cur = min(len, PAGE_SIZE - offset);
>  		kaddr = page_address(page);
> @@ -4462,34 +4465,21 @@ void memcpy_extent_buffer(const struct extent_buffer *dst,
>  			  unsigned long dst_offset, unsigned long src_offset,
>  			  unsigned long len)
>  {
> -	size_t cur;
> -	size_t dst_off_in_page;
> -	size_t src_off_in_page;
> -	unsigned long dst_i;
> -	unsigned long src_i;
> +	unsigned long cur = src_offset;
>  
>  	if (check_eb_range(dst, dst_offset, len) ||
>  	    check_eb_range(dst, src_offset, len))
>  		return;
>  
> -	while (len > 0) {
> -		dst_off_in_page = get_eb_offset_in_page(dst, dst_offset);
> -		src_off_in_page = get_eb_offset_in_page(dst, src_offset);
> +	while (cur < src_offset + len) {
> +		int index = get_eb_page_index(cur);

get_eb_page_index() returns unsigned long, so this should be unified.
Fixed in the commit.

> +		unsigned long offset = get_eb_offset_in_page(dst, cur);
> +		unsigned long cur_len = min(src_offset + len - cur, PAGE_SIZE - offset);
> +		unsigned long offset_to_start = cur - src_offset;
> +		void *src_addr = page_address(dst->pages[index]) + offset;
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 4e252fd7b78a..4db90ede8219 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4183,6 +4183,8 @@  void write_extent_buffer(const struct extent_buffer *eb, const void *srcv,
 	struct page *page;
 	char *kaddr;
 	char *src = (char *)srcv;
+	/* For unmapped (dummy) ebs, no need to check their uptodate status. */
+	bool check_uptodate = !test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags);
 	unsigned long i = get_eb_page_index(start);
 
 	WARN_ON(test_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags));
@@ -4194,7 +4196,8 @@  void write_extent_buffer(const struct extent_buffer *eb, const void *srcv,
 
 	while (len > 0) {
 		page = eb->pages[i];
-		assert_eb_page_uptodate(eb, page);
+		if (check_uptodate)
+			assert_eb_page_uptodate(eb, page);
 
 		cur = min(len, PAGE_SIZE - offset);
 		kaddr = page_address(page);
@@ -4462,34 +4465,21 @@  void memcpy_extent_buffer(const struct extent_buffer *dst,
 			  unsigned long dst_offset, unsigned long src_offset,
 			  unsigned long len)
 {
-	size_t cur;
-	size_t dst_off_in_page;
-	size_t src_off_in_page;
-	unsigned long dst_i;
-	unsigned long src_i;
+	unsigned long cur = src_offset;
 
 	if (check_eb_range(dst, dst_offset, len) ||
 	    check_eb_range(dst, src_offset, len))
 		return;
 
-	while (len > 0) {
-		dst_off_in_page = get_eb_offset_in_page(dst, dst_offset);
-		src_off_in_page = get_eb_offset_in_page(dst, src_offset);
+	while (cur < src_offset + len) {
+		int index = get_eb_page_index(cur);
+		unsigned long offset = get_eb_offset_in_page(dst, cur);
+		unsigned long cur_len = min(src_offset + len - cur, PAGE_SIZE - offset);
+		unsigned long offset_to_start = cur - src_offset;
+		void *src_addr = page_address(dst->pages[index]) + offset;
 
-		dst_i = get_eb_page_index(dst_offset);
-		src_i = get_eb_page_index(src_offset);
-
-		cur = min(len, (unsigned long)(PAGE_SIZE -
-					       src_off_in_page));
-		cur = min_t(unsigned long, cur,
-			(unsigned long)(PAGE_SIZE - dst_off_in_page));
-
-		copy_pages(dst->pages[dst_i], dst->pages[src_i],
-			   dst_off_in_page, src_off_in_page, cur);
-
-		src_offset += cur;
-		dst_offset += cur;
-		len -= cur;
+		write_extent_buffer(dst, src_addr, dst_offset + offset_to_start, cur_len);
+		cur += cur_len;
 	}
 }