small btrfs-zoned fixlets and optimizations

small btrfs-zoned fixlets and optimizations

[Christoph Hellwig]

Hi all,

this fixes a minor correctness issue and adds some minor
optimizations to the btrfs zoned code.

It sits on top of the

  "consolidate btrfs checksumming, repair and bio splitting v2"

series.

Diffstat:
 fs/btrfs/bio.c                    |    2 -
 fs/btrfs/block-group.c            |    9 +-----
 fs/btrfs/block-group.h            |    3 --
 fs/btrfs/extent_io.c              |   10 ++----
 fs/btrfs/inode.c                  |   22 +++++++-------
 fs/btrfs/ordered-data.c           |   10 +++---
 fs/btrfs/ordered-data.h           |    3 --
 fs/btrfs/relocation.c             |    2 -
 fs/btrfs/tests/extent-map-tests.c |    2 -
 fs/btrfs/zoned.c                  |   56 +++++++++++++++++++++-----------------
 fs/btrfs/zoned.h                  |    4 +-
 include/trace/events/btrfs.h      |    2 -
 12 files changed, 61 insertions(+), 64 deletions(-)

[PATCH 1/7] btrfs: use file_offset to limit bios size in calc_bio_boundaries

[Christoph Hellwig]

btrfs_ordered_extent->disk_bytenr can be rewritten by the zoned I/O
completion handler, and thus in general is not a good idea to limit I/O
size.  But the maximum bio size calculation can easily be done using the
file_offset fields in the btrfs_ordered_extent and btrfs_bio structures,
so switch to that instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a65a1629d3356d..1fcb55e549717f 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -944,8 +944,8 @@ static void calc_bio_boundaries(struct btrfs_bio_ctrl *bio_ctrl,
 		ordered = btrfs_lookup_ordered_extent(inode, file_offset);
 		if (ordered) {
 			bio_ctrl->len_to_oe_boundary = min_t(u32, U32_MAX,
-					ordered->disk_bytenr +
-					ordered->disk_num_bytes - logical);
+					ordered->file_offset +
+					ordered->disk_num_bytes - file_offset);
 			btrfs_put_ordered_extent(ordered);
 			return;
 		}
-- 
2.35.1

Re: [PATCH 1/7] btrfs: use file_offset to limit bios size in calc_bio_boundaries

[Josef Bacik]

On Mon, Dec 12, 2022 at 08:37:18AM +0100, Christoph Hellwig wrote:
> btrfs_ordered_extent->disk_bytenr can be rewritten by the zoned I/O
> completion handler, and thus in general is not a good idea to limit I/O
> size.  But the maximum bio size calculation can easily be done using the
> file_offset fields in the btrfs_ordered_extent and btrfs_bio structures,
> so switch to that instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Can you add a comment to the code as well?  I'm going to see this in a year or two
and spend a little bit scratching my head as to why we're using the file_offset
here.  With that done you can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH 1/7] btrfs: use file_offset to limit bios size in calc_bio_boundaries

[Christoph Hellwig]

On Mon, Dec 12, 2022 at 01:54:14PM -0500, Josef Bacik wrote:
> Can you add a comment to the code as well?

Sure.

> I'm going to see this in a year or two
> and spend a little bit scratching my head as to why we're using the file_offset
> here.  With that done you can add

Heh.  I hope the code in this form is actually gone in a year, as the
concept of first creating the ordered extents and inserting them into a
tree, and then doing a lookup a few lines down in writepages is not
exactly very efficient and we should be able to just pass a pointer
down.  But that is several series down in my queue.

[PATCH 2/7] btrfs; rename the disk_bytenr in strut btrfs_ordered_extent

[Christoph Hellwig]

Rename the disk_bytenr field to logical to make it clear it holds
a btrfs logical address.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c             | 18 +++++++++---------
 fs/btrfs/ordered-data.c      | 10 +++++-----
 fs/btrfs/ordered-data.h      |  2 +-
 fs/btrfs/relocation.c        |  2 +-
 fs/btrfs/zoned.c             |  4 ++--
 include/trace/events/btrfs.h |  2 +-
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 373b7281f5c7dd..3a3a3e0e9484c6 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2667,9 +2667,9 @@ blk_status_t btrfs_extract_ordered_extent(struct btrfs_bio *bbio)
 		goto out;
 	}
 
-	ordered_end = ordered->disk_bytenr + ordered->disk_num_bytes;
+	ordered_end = ordered->logical + ordered->disk_num_bytes;
 	/* bio must be in one ordered extent */
-	if (WARN_ON_ONCE(start < ordered->disk_bytenr || end > ordered_end)) {
+	if (WARN_ON_ONCE(start < ordered->logical || end > ordered_end)) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2681,7 +2681,7 @@ blk_status_t btrfs_extract_ordered_extent(struct btrfs_bio *bbio)
 	}
 
 	file_len = ordered->num_bytes;
-	pre = start - ordered->disk_bytenr;
+	pre = start - ordered->logical;
 	post = ordered_end - end;
 
 	ret = btrfs_split_ordered_extent(ordered, pre, post);
@@ -3094,7 +3094,7 @@ static int insert_ordered_extent_file_extent(struct btrfs_trans_handle *trans,
 
 	memset(&stack_fi, 0, sizeof(stack_fi));
 	btrfs_set_stack_file_extent_type(&stack_fi, BTRFS_FILE_EXTENT_REG);
-	btrfs_set_stack_file_extent_disk_bytenr(&stack_fi, oe->disk_bytenr);
+	btrfs_set_stack_file_extent_disk_bytenr(&stack_fi, oe->logical);
 	btrfs_set_stack_file_extent_disk_num_bytes(&stack_fi,
 						   oe->disk_num_bytes);
 	btrfs_set_stack_file_extent_offset(&stack_fi, oe->offset);
@@ -3165,7 +3165,7 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 	/* A valid bdev implies a write on a sequential zone */
 	if (ordered_extent->bdev) {
 		btrfs_rewrite_logical_zoned(ordered_extent);
-		btrfs_zone_finish_endio(fs_info, ordered_extent->disk_bytenr,
+		btrfs_zone_finish_endio(fs_info, ordered_extent->logical,
 					ordered_extent->disk_num_bytes);
 	}
 
@@ -3220,7 +3220,7 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 						ordered_extent->file_offset,
 						ordered_extent->file_offset +
 						logical_len);
-		btrfs_zoned_release_data_reloc_bg(fs_info, ordered_extent->disk_bytenr,
+		btrfs_zoned_release_data_reloc_bg(fs_info, ordered_extent->logical,
 						  ordered_extent->disk_num_bytes);
 	} else {
 		BUG_ON(root == fs_info->tree_root);
@@ -3228,7 +3228,7 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 		if (!ret) {
 			clear_reserved_extent = false;
 			btrfs_release_delalloc_bytes(fs_info,
-						ordered_extent->disk_bytenr,
+						ordered_extent->logical,
 						ordered_extent->disk_num_bytes);
 		}
 	}
@@ -3312,11 +3312,11 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 			 */
 			if (ret && btrfs_test_opt(fs_info, DISCARD_SYNC))
 				btrfs_discard_extent(fs_info,
-						ordered_extent->disk_bytenr,
+						ordered_extent->logical,
 						ordered_extent->disk_num_bytes,
 						NULL);
 			btrfs_free_reserved_extent(fs_info,
-					ordered_extent->disk_bytenr,
+					ordered_extent->logical,
 					ordered_extent->disk_num_bytes, 1);
 		}
 	}
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 4bed0839b64033..72e1acd4624169 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -199,7 +199,7 @@ int btrfs_add_ordered_extent(struct btrfs_inode *inode, u64 file_offset,
 	entry->file_offset = file_offset;
 	entry->num_bytes = num_bytes;
 	entry->ram_bytes = ram_bytes;
-	entry->disk_bytenr = disk_bytenr;
+	entry->logical = disk_bytenr;
 	entry->disk_num_bytes = disk_num_bytes;
 	entry->offset = offset;
 	entry->bytes_left = num_bytes;
@@ -642,8 +642,8 @@ u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 		ordered = list_first_entry(&splice, struct btrfs_ordered_extent,
 					   root_extent_list);
 
-		if (range_end <= ordered->disk_bytenr ||
-		    ordered->disk_bytenr + ordered->disk_num_bytes <= range_start) {
+		if (range_end <= ordered->logical ||
+		    ordered->logical + ordered->disk_num_bytes <= range_start) {
 			list_move_tail(&ordered->root_extent_list, &skipped);
 			cond_resched_lock(&root->ordered_extent_lock);
 			continue;
@@ -1098,7 +1098,7 @@ static int clone_ordered_extent(struct btrfs_ordered_extent *ordered, u64 pos,
 	struct inode *inode = ordered->inode;
 	struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info;
 	u64 file_offset = ordered->file_offset + pos;
-	u64 disk_bytenr = ordered->disk_bytenr + pos;
+	u64 disk_bytenr = ordered->logical + pos;
 	unsigned long flags = ordered->flags & BTRFS_ORDERED_TYPE_FLAGS;
 
 	/*
@@ -1133,7 +1133,7 @@ int btrfs_split_ordered_extent(struct btrfs_ordered_extent *ordered, u64 pre,
 		tree->last = NULL;
 
 	ordered->file_offset += pre;
-	ordered->disk_bytenr += pre;
+	ordered->logical += pre;
 	ordered->num_bytes -= (pre + post);
 	ordered->disk_num_bytes -= (pre + post);
 	ordered->bytes_left -= (pre + post);
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 89f82b78f590f3..71f200b14a9369 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -96,7 +96,7 @@ struct btrfs_ordered_extent {
 	 */
 	u64 num_bytes;
 	u64 ram_bytes;
-	u64 disk_bytenr;
+	u64 logical;
 	u64 disk_num_bytes;
 	u64 offset;
 
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 31ec4a7658ce6f..798e0e01490dd8 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4379,7 +4379,7 @@ int btrfs_reloc_clone_csums(struct btrfs_inode *inode, u64 file_pos, u64 len)
 		 * disk_len vs real len like with real inodes since it's all
 		 * disk length.
 		 */
-		new_bytenr = ordered->disk_bytenr + sums->bytenr - disk_bytenr;
+		new_bytenr = ordered->logical + sums->bytenr - disk_bytenr;
 		sums->bytenr = new_bytenr;
 
 		btrfs_add_ordered_sum(ordered, sums);
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 0b769dc8bcdac0..e8681615324dcb 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1678,7 +1678,7 @@ void btrfs_rewrite_logical_zoned(struct btrfs_ordered_extent *ordered)
 	struct extent_map_tree *em_tree;
 	struct extent_map *em;
 	struct btrfs_ordered_sum *sum;
-	u64 orig_logical = ordered->disk_bytenr;
+	u64 orig_logical = ordered->logical;
 	u64 *logical = NULL;
 	int nr, stripe_len;
 
@@ -1697,7 +1697,7 @@ void btrfs_rewrite_logical_zoned(struct btrfs_ordered_extent *ordered)
 	if (orig_logical == *logical)
 		goto out;
 
-	ordered->disk_bytenr = *logical;
+	ordered->logical = *logical;
 
 	em_tree = &inode->extent_tree;
 	write_lock(&em_tree->lock);
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 0bce0b4ff2faf3..58ea7be57b010e 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -536,7 +536,7 @@ DECLARE_EVENT_CLASS(btrfs__ordered_extent,
 	TP_fast_assign_btrfs(inode->root->fs_info,
 		__entry->ino 		= btrfs_ino(inode);
 		__entry->file_offset	= ordered->file_offset;
-		__entry->start		= ordered->disk_bytenr;
+		__entry->start		= ordered->logical;
 		__entry->len		= ordered->num_bytes;
 		__entry->disk_len	= ordered->disk_num_bytes;
 		__entry->bytes_left	= ordered->bytes_left;
-- 
2.35.1

Re: [PATCH 2/7] btrfs; rename the disk_bytenr in strut btrfs_ordered_extent

[Josef Bacik]

On Mon, Dec 12, 2022 at 08:37:19AM +0100, Christoph Hellwig wrote:
> Rename the disk_bytenr field to logical to make it clear it holds
> a btrfs logical address.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/btrfs/inode.c             | 18 +++++++++---------
>  fs/btrfs/ordered-data.c      | 10 +++++-----
>  fs/btrfs/ordered-data.h      |  2 +-
>  fs/btrfs/relocation.c        |  2 +-
>  fs/btrfs/zoned.c             |  4 ++--
>  include/trace/events/btrfs.h |  2 +-
>  6 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 373b7281f5c7dd..3a3a3e0e9484c6 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2667,9 +2667,9 @@ blk_status_t btrfs_extract_ordered_extent(struct btrfs_bio *bbio)
>  		goto out;
>  	}
>  
> -	ordered_end = ordered->disk_bytenr + ordered->disk_num_bytes;
> +	ordered_end = ordered->logical + ordered->disk_num_bytes;
>  	/* bio must be in one ordered extent */
> -	if (WARN_ON_ONCE(start < ordered->disk_bytenr || end > ordered_end)) {
> +	if (WARN_ON_ONCE(start < ordered->logical || end > ordered_end)) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -2681,7 +2681,7 @@ blk_status_t btrfs_extract_ordered_extent(struct btrfs_bio *bbio)
>  	}
>  
>  	file_len = ordered->num_bytes;
> -	pre = start - ordered->disk_bytenr;
> +	pre = start - ordered->logical;
>  	post = ordered_end - end;
>  
>  	ret = btrfs_split_ordered_extent(ordered, pre, post);
> @@ -3094,7 +3094,7 @@ static int insert_ordered_extent_file_extent(struct btrfs_trans_handle *trans,
>  
>  	memset(&stack_fi, 0, sizeof(stack_fi));
>  	btrfs_set_stack_file_extent_type(&stack_fi, BTRFS_FILE_EXTENT_REG);
> -	btrfs_set_stack_file_extent_disk_bytenr(&stack_fi, oe->disk_bytenr);
> +	btrfs_set_stack_file_extent_disk_bytenr(&stack_fi, oe->logical);
>  	btrfs_set_stack_file_extent_disk_num_bytes(&stack_fi,
>  						   oe->disk_num_bytes);
>  	btrfs_set_stack_file_extent_offset(&stack_fi, oe->offset);
> @@ -3165,7 +3165,7 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
>  	/* A valid bdev implies a write on a sequential zone */
>  	if (ordered_extent->bdev) {
>  		btrfs_rewrite_logical_zoned(ordered_extent);
> -		btrfs_zone_finish_endio(fs_info, ordered_extent->disk_bytenr,
> +		btrfs_zone_finish_endio(fs_info, ordered_extent->logical,
>  					ordered_extent->disk_num_bytes);
>  	}
>  
> @@ -3220,7 +3220,7 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
>  						ordered_extent->file_offset,
>  						ordered_extent->file_offset +
>  						logical_len);
> -		btrfs_zoned_release_data_reloc_bg(fs_info, ordered_extent->disk_bytenr,
> +		btrfs_zoned_release_data_reloc_bg(fs_info, ordered_extent->logical,
>  						  ordered_extent->disk_num_bytes);
>  	} else {
>  		BUG_ON(root == fs_info->tree_root);
> @@ -3228,7 +3228,7 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
>  		if (!ret) {
>  			clear_reserved_extent = false;
>  			btrfs_release_delalloc_bytes(fs_info,
> -						ordered_extent->disk_bytenr,
> +						ordered_extent->logical,
>  						ordered_extent->disk_num_bytes);
>  		}
>  	}
> @@ -3312,11 +3312,11 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
>  			 */
>  			if (ret && btrfs_test_opt(fs_info, DISCARD_SYNC))
>  				btrfs_discard_extent(fs_info,
> -						ordered_extent->disk_bytenr,
> +						ordered_extent->logical,
>  						ordered_extent->disk_num_bytes,
>  						NULL);
>  			btrfs_free_reserved_extent(fs_info,
> -					ordered_extent->disk_bytenr,
> +					ordered_extent->logical,
>  					ordered_extent->disk_num_bytes, 1);
>  		}
>  	}
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 4bed0839b64033..72e1acd4624169 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -199,7 +199,7 @@ int btrfs_add_ordered_extent(struct btrfs_inode *inode, u64 file_offset,
>  	entry->file_offset = file_offset;
>  	entry->num_bytes = num_bytes;
>  	entry->ram_bytes = ram_bytes;
> -	entry->disk_bytenr = disk_bytenr;
> +	entry->logical = disk_bytenr;
>  	entry->disk_num_bytes = disk_num_bytes;
>  	entry->offset = offset;
>  	entry->bytes_left = num_bytes;
> @@ -642,8 +642,8 @@ u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
>  		ordered = list_first_entry(&splice, struct btrfs_ordered_extent,
>  					   root_extent_list);
>  
> -		if (range_end <= ordered->disk_bytenr ||
> -		    ordered->disk_bytenr + ordered->disk_num_bytes <= range_start) {
> +		if (range_end <= ordered->logical ||
> +		    ordered->logical + ordered->disk_num_bytes <= range_start) {
>  			list_move_tail(&ordered->root_extent_list, &skipped);
>  			cond_resched_lock(&root->ordered_extent_lock);
>  			continue;
> @@ -1098,7 +1098,7 @@ static int clone_ordered_extent(struct btrfs_ordered_extent *ordered, u64 pos,
>  	struct inode *inode = ordered->inode;
>  	struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info;
>  	u64 file_offset = ordered->file_offset + pos;
> -	u64 disk_bytenr = ordered->disk_bytenr + pos;
> +	u64 disk_bytenr = ordered->logical + pos;
>  	unsigned long flags = ordered->flags & BTRFS_ORDERED_TYPE_FLAGS;
>  
>  	/*
> @@ -1133,7 +1133,7 @@ int btrfs_split_ordered_extent(struct btrfs_ordered_extent *ordered, u64 pre,
>  		tree->last = NULL;
>  
>  	ordered->file_offset += pre;
> -	ordered->disk_bytenr += pre;
> +	ordered->logical += pre;
>  	ordered->num_bytes -= (pre + post);
>  	ordered->disk_num_bytes -= (pre + post);
>  	ordered->bytes_left -= (pre + post);
> diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
> index 89f82b78f590f3..71f200b14a9369 100644
> --- a/fs/btrfs/ordered-data.h
> +++ b/fs/btrfs/ordered-data.h
> @@ -96,7 +96,7 @@ struct btrfs_ordered_extent {
>  	 */
>  	u64 num_bytes;
>  	u64 ram_bytes;
> -	u64 disk_bytenr;
> +	u64 logical;

There's a comment above here that reads

        /*                                                                                                                            
         * These fields directly correspond to the same fields in                                                                     
         * btrfs_file_extent_item.
         */

which with this change is no longer true.  Please update the comment
appropriately, other than that the change is reasonable to me, you can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

once you've updated the patch.  Thanks,

Josef

Re: [PATCH 2/7] btrfs; rename the disk_bytenr in strut btrfs_ordered_extent

[Christoph Hellwig]

On Mon, Dec 12, 2022 at 01:58:06PM -0500, Josef Bacik wrote:
>         /*                                                                                                                            
>          * These fields directly correspond to the same fields in                                                                     
>          * btrfs_file_extent_item.
>          */
> 
> which with this change is no longer true.  Please update the comment
> appropriately, other than that the change is reasonable to me, you can add

Ok.  Or should we skip this patch and stick to the on-disk naming
even if it is a bit confusing?

Re: [PATCH 2/7] btrfs; rename the disk_bytenr in strut btrfs_ordered_extent

[Josef Bacik]

On Tue, Dec 13, 2022 at 03:08:49PM +0100, Christoph Hellwig wrote:
> On Mon, Dec 12, 2022 at 01:58:06PM -0500, Josef Bacik wrote:
> >         /*                                                                                                                            
> >          * These fields directly correspond to the same fields in                                                                     
> >          * btrfs_file_extent_item.
> >          */
> > 
> > which with this change is no longer true.  Please update the comment
> > appropriately, other than that the change is reasonable to me, you can add
> 
> Ok.  Or should we skip this patch and stick to the on-disk naming
> even if it is a bit confusing?

My preference is to leave it, but generally for these things I defer to
outsiders, just because I'm used to it doesn't mean it's right.  In this case
because it maps to the on-disk naming I'd rather leave it unless you feel
strongly.  Thanks,

Josef

[PATCH 3/7] btrfs: set bbio->file_offset in alloc_new_bio

[Christoph Hellwig]

Instead of digging into the bio_vec in submit_one_bio, set file_offset at
bio allocation time from the provided parameter.  This also ensures that
the file_offset is available all the time when building up the bio
payload.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 1fcb55e549717f..3d1bcc71ee9998 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -130,8 +130,6 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
 	/* Caller should ensure the bio has at least some range added */
 	ASSERT(bio->bi_iter.bi_size);
 
-	btrfs_bio(bio)->file_offset = page_offset(bv->bv_page) + bv->bv_offset;
-
 	if (!is_data_inode(inode))
 		bio->bi_opf |= REQ_META;
 
@@ -973,6 +971,7 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 		bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 	else
 		bio->bi_iter.bi_sector = (disk_bytenr + offset) >> SECTOR_SHIFT;
+	btrfs_bio(bio)->file_offset = file_offset;
 	bio_ctrl->bio = bio;
 	bio_ctrl->compress_type = compress_type;
 	calc_bio_boundaries(bio_ctrl, inode, file_offset);
-- 
2.35.1

[PATCH 4/7] btrfs: pass a btrfs_bio to btrfs_use_append

[Christoph Hellwig]

struct btrfs_bio has all the information needed for btrfs_use_append, so
pass that instead of a btrfs_inode and file_offset.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/bio.c       | 2 +-
 fs/btrfs/extent_io.c | 3 +--
 fs/btrfs/zoned.c     | 4 +++-
 fs/btrfs/zoned.h     | 4 ++--
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 4ccbc120e869ba..d4ce7c83f85a25 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -614,7 +614,7 @@ static bool btrfs_submit_chunk(struct bio *bio, int mirror_num)
 	u64 logical = bio->bi_iter.bi_sector << 9;
 	u64 length = bio->bi_iter.bi_size;
 	u64 map_length = length;
-	bool use_append = btrfs_use_zone_append(inode, logical);
+	bool use_append = btrfs_use_zone_append(bbio);
 	struct btrfs_io_context *bioc = NULL;
 	struct btrfs_io_stripe smap;
 	blk_status_t ret;
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 3d1bcc71ee9998..92724fac9e0824 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -930,7 +930,6 @@ static void calc_bio_boundaries(struct btrfs_bio_ctrl *bio_ctrl,
 				struct btrfs_inode *inode, u64 file_offset)
 {
 	struct btrfs_ordered_extent *ordered;
-	u64 logical = (bio_ctrl->bio->bi_iter.bi_sector << SECTOR_SHIFT);
 
 	/*
 	 * Limit the extent to the ordered boundary for Zone Append.
@@ -938,7 +937,7 @@ static void calc_bio_boundaries(struct btrfs_bio_ctrl *bio_ctrl,
 	 * to them.
 	 */
 	if (bio_ctrl->compress_type == BTRFS_COMPRESS_NONE &&
-	    btrfs_use_zone_append(inode, logical)) {
+	    btrfs_use_zone_append(btrfs_bio(bio_ctrl->bio))) {
 		ordered = btrfs_lookup_ordered_extent(inode, file_offset);
 		if (ordered) {
 			bio_ctrl->len_to_oe_boundary = min_t(u32, U32_MAX,
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index e8681615324dcb..77f453005dc924 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1622,8 +1622,10 @@ void btrfs_free_redirty_list(struct btrfs_transaction *trans)
 	spin_unlock(&trans->releasing_ebs_lock);
 }
 
-bool btrfs_use_zone_append(struct btrfs_inode *inode, u64 start)
+bool btrfs_use_zone_append(struct btrfs_bio *bbio)
 {
+	u64 start = (bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT);
+	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct btrfs_block_group *cache;
 	bool ret = false;
diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
index 157f46132c56e0..c0570d35fea291 100644
--- a/fs/btrfs/zoned.h
+++ b/fs/btrfs/zoned.h
@@ -55,7 +55,7 @@ void btrfs_calc_zone_unusable(struct btrfs_block_group *cache);
 void btrfs_redirty_list_add(struct btrfs_transaction *trans,
 			    struct extent_buffer *eb);
 void btrfs_free_redirty_list(struct btrfs_transaction *trans);
-bool btrfs_use_zone_append(struct btrfs_inode *inode, u64 start);
+bool btrfs_use_zone_append(struct btrfs_bio *bbio);
 void btrfs_record_physical_zoned(struct btrfs_bio *bbio);
 void btrfs_rewrite_logical_zoned(struct btrfs_ordered_extent *ordered);
 bool btrfs_check_meta_write_pointer(struct btrfs_fs_info *fs_info,
@@ -181,7 +181,7 @@ static inline void btrfs_redirty_list_add(struct btrfs_transaction *trans,
 					  struct extent_buffer *eb) { }
 static inline void btrfs_free_redirty_list(struct btrfs_transaction *trans) { }
 
-static inline bool btrfs_use_zone_append(struct btrfs_inode *inode, u64 start)
+static inline bool btrfs_use_zone_append(struct btrfs_bio *bbio)
 {
 	return false;
 }
-- 
2.35.1

Re: [PATCH 4/7] btrfs: pass a btrfs_bio to btrfs_use_append

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 5/7] btrfs: never return true for reads in btrfs_use_zone_append

[Christoph Hellwig]

Using Zone Append only makes sense for writes to the device, so check
that in btrfs_use_zone_append.  This avoids the possibility of
artificially limited read size on zoned file systems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/zoned.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 77f453005dc924..af063089ad3465 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1636,6 +1636,9 @@ bool btrfs_use_zone_append(struct btrfs_bio *bbio)
 	if (!is_data_inode(&inode->vfs_inode))
 		return false;
 
+	if (btrfs_op(&bbio->bio) != BTRFS_MAP_WRITE)
+		return false;
+
 	/*
 	 * Using REQ_OP_ZONE_APPNED for relocation can break assumptions on the
 	 * extent layout the relocation code has.
-- 
2.35.1

Re: [PATCH 5/7] btrfs: never return true for reads in btrfs_use_zone_append

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 6/7] btrfs: don't rely on unchanging ->bi_bdev for zone append remaps

[Christoph Hellwig]

btrfs_record_physical_zoned relies on a bio->bi_bdev samples in the
bio_end_io handler to find the reverse map for remapping the zone append
write, but stacked block device drivers can and usually do change bi_bdev
when sending on the bio to a lower device.  This can happen e.g. with the
nvme-multipath driver when a NVMe SSD sets the shared namespace bit.

But there is no real need for the bdev in btrfs_record_physical_zoned,
as it is only passed to btrfs_rmap_block, which uses it to pick the
mapping to report if there are multiple reverse mappings.  As zone
writes can only do simple non-mirror writes right now, and anything
more complex will use the stripe tree there is no chance of the multiple
mappings case actually happening.

Instead open code the subset of btrfs_rmap_block in
btrfs_record_physical_zoned, which also removes a memory allocation and
remove the bdev field in the ordered extent.

Fixes: d8e3fb106f39 ("btrfs: zoned: use ZONE_APPEND write for zoned mode")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c        |  4 ++--
 fs/btrfs/ordered-data.h |  1 -
 fs/btrfs/zoned.c        | 47 +++++++++++++++++++++--------------------
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3a3a3e0e9484c6..94749227a73616 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3162,8 +3162,8 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 		goto out;
 	}
 
-	/* A valid bdev implies a write on a sequential zone */
-	if (ordered_extent->bdev) {
+	/* A valid ->physical implies a write on a sequential zone */
+	if (ordered_extent->physical != (u64)-1) {
 		btrfs_rewrite_logical_zoned(ordered_extent);
 		btrfs_zone_finish_endio(fs_info, ordered_extent->logical,
 					ordered_extent->disk_num_bytes);
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 71f200b14a9369..7ab06aaeae83a1 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -157,7 +157,6 @@ struct btrfs_ordered_extent {
 	 * command in a workqueue context
 	 */
 	u64 physical;
-	struct block_device *bdev;
 };
 
 static inline void
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index af063089ad3465..9782a40985923f 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1671,8 +1671,6 @@ void btrfs_record_physical_zoned(struct btrfs_bio *bbio)
 		return;
 
 	ordered->physical = physical;
-	ordered->bdev = bbio->bio.bi_bdev;
-
 	btrfs_put_ordered_extent(ordered);
 }
 
@@ -1684,43 +1682,46 @@ void btrfs_rewrite_logical_zoned(struct btrfs_ordered_extent *ordered)
 	struct extent_map *em;
 	struct btrfs_ordered_sum *sum;
 	u64 orig_logical = ordered->logical;
-	u64 *logical = NULL;
-	int nr, stripe_len;
+	struct map_lookup *map;
+	u64 physical = ordered->physical;
+	u64 chunk_start_phys;
+	u64 logical;
 
-	/* Zoned devices should not have partitions. So, we can assume it is 0 */
-	ASSERT(!bdev_is_partition(ordered->bdev));
-	if (WARN_ON(!ordered->bdev))
+	em = btrfs_get_chunk_map(fs_info, orig_logical, 1);
+	if (IS_ERR(em))
 		return;
+	map = em->map_lookup;
+	chunk_start_phys = map->stripes[0].physical;
 
-	if (WARN_ON(btrfs_rmap_block(fs_info, orig_logical, ordered->bdev,
-				     ordered->physical, &logical, &nr,
-				     &stripe_len)))
-		goto out;
-
-	WARN_ON(nr != 1);
+	if (WARN_ON_ONCE(map->num_stripes > 1) ||
+	    WARN_ON_ONCE((map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK) != 0) ||
+	    WARN_ON_ONCE(physical < chunk_start_phys) ||
+	    WARN_ON_ONCE(physical > chunk_start_phys + em->orig_block_len)) {
+		free_extent_map(em);
+		return;
+	}
+	logical = em->start + (physical - map->stripes[0].physical);
+	free_extent_map(em);
 
-	if (orig_logical == *logical)
-		goto out;
+	if (orig_logical == logical)
+		return;
 
-	ordered->logical = *logical;
+	ordered->logical = logical;
 
 	em_tree = &inode->extent_tree;
 	write_lock(&em_tree->lock);
 	em = search_extent_mapping(em_tree, ordered->file_offset,
 				   ordered->num_bytes);
-	em->block_start = *logical;
+	em->block_start = logical;
 	free_extent_map(em);
 	write_unlock(&em_tree->lock);
 
 	list_for_each_entry(sum, &ordered->list, list) {
-		if (*logical < orig_logical)
-			sum->bytenr -= orig_logical - *logical;
+		if (logical < orig_logical)
+			sum->bytenr -= orig_logical - logical;
 		else
-			sum->bytenr += *logical - orig_logical;
+			sum->bytenr += logical - orig_logical;
 	}
-
-out:
-	kfree(logical);
 }
 
 bool btrfs_check_meta_write_pointer(struct btrfs_fs_info *fs_info,
-- 
2.35.1

[PATCH 7/7] btrfs: remove the bdev argument to btrfs_rmap_block

[Christoph Hellwig]

The only user in the zoned remap code is gone now, so remove the argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/block-group.c            | 9 ++-------
 fs/btrfs/block-group.h            | 3 +--
 fs/btrfs/tests/extent-map-tests.c | 2 +-
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 708d843daa72de..72688a4f38af99 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1816,7 +1816,6 @@ static void set_avail_alloc_bits(struct btrfs_fs_info *fs_info, u64 flags)
  *
  * @fs_info:       the filesystem
  * @chunk_start:   logical address of block group
- * @bdev:	   physical device to resolve, can be NULL to indicate any device
  * @physical:	   physical address to map to logical addresses
  * @logical:	   return array of logical addresses which map to @physical
  * @naddrs:	   length of @logical
@@ -1827,8 +1826,7 @@ static void set_avail_alloc_bits(struct btrfs_fs_info *fs_info, u64 flags)
  * block copies.
  */
 int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
-		     struct block_device *bdev, u64 physical, u64 **logical,
-		     int *naddrs, int *stripe_len)
+		     u64 physical, u64 **logical, int *naddrs, int *stripe_len)
 {
 	struct extent_map *em;
 	struct map_lookup *map;
@@ -1868,9 +1866,6 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
 			      data_stripe_length))
 			continue;
 
-		if (bdev && map->stripes[i].dev->bdev != bdev)
-			continue;
-
 		stripe_nr = physical - map->stripes[i].physical;
 		stripe_nr = div64_u64_rem(stripe_nr, map->stripe_len, &offset);
 
@@ -1927,7 +1922,7 @@ static int exclude_super_stripes(struct btrfs_block_group *cache)
 
 	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
-		ret = btrfs_rmap_block(fs_info, cache->start, NULL,
+		ret = btrfs_rmap_block(fs_info, cache->start,
 				       bytenr, &logical, &nr, &stripe_len);
 		if (ret)
 			return ret;
diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index a02ea76fd6cffe..0462f9b5cedf95 100644
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -315,8 +315,7 @@ u64 btrfs_get_alloc_profile(struct btrfs_fs_info *fs_info, u64 orig_flags);
 void btrfs_put_block_group_cache(struct btrfs_fs_info *info);
 int btrfs_free_block_groups(struct btrfs_fs_info *info);
 int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
-		       struct block_device *bdev, u64 physical, u64 **logical,
-		       int *naddrs, int *stripe_len);
+		     u64 physical, u64 **logical, int *naddrs, int *stripe_len);
 
 static inline u64 btrfs_data_alloc_profile(struct btrfs_fs_info *fs_info)
 {
diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c
index c5b3a631bf4fb4..f2f2e11dac4c02 100644
--- a/fs/btrfs/tests/extent-map-tests.c
+++ b/fs/btrfs/tests/extent-map-tests.c
@@ -509,7 +509,7 @@ static int test_rmap_block(struct btrfs_fs_info *fs_info,
 		goto out_free;
 	}
 
-	ret = btrfs_rmap_block(fs_info, em->start, NULL, btrfs_sb_offset(1),
+	ret = btrfs_rmap_block(fs_info, em->start, btrfs_sb_offset(1),
 			       &logical, &out_ndaddrs, &out_stripe_len);
 	if (ret || (out_ndaddrs == 0 && test->expected_mapped_addr)) {
 		test_err("didn't rmap anything but expected %d",
-- 
2.35.1

Re: small btrfs-zoned fixlets and optimizations

[Josef Bacik]

On Mon, Dec 12, 2022 at 08:37:17AM +0100, Christoph Hellwig wrote:
> Hi all,
> 
> this fixes a minor correctness issue and adds some minor
> optimizations to the btrfs zoned code.
> 
> It sits on top of the
> 
>   "consolidate btrfs checksumming, repair and bio splitting v2"
> 
> series.
> 

I had a couple of comments, the rest you can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: small btrfs-zoned fixlets and optimizations

[David Sterba]

On Mon, Dec 12, 2022 at 08:37:17AM +0100, Christoph Hellwig wrote:
> Hi all,
> 
> this fixes a minor correctness issue and adds some minor
> optimizations to the btrfs zoned code.
> 
> It sits on top of the
> 
>   "consolidate btrfs checksumming, repair and bio splitting v2"
> 
> series.
> 
> Diffstat:
>  fs/btrfs/bio.c                    |    2 -
>  fs/btrfs/block-group.c            |    9 +-----
>  fs/btrfs/block-group.h            |    3 --
>  fs/btrfs/extent_io.c              |   10 ++----
>  fs/btrfs/inode.c                  |   22 +++++++-------
>  fs/btrfs/ordered-data.c           |   10 +++---
>  fs/btrfs/ordered-data.h           |    3 --
>  fs/btrfs/relocation.c             |    2 -
>  fs/btrfs/tests/extent-map-tests.c |    2 -
>  fs/btrfs/zoned.c                  |   56 +++++++++++++++++++++-----------------
>  fs/btrfs/zoned.h                  |    4 +-
>  include/trace/events/btrfs.h      |    2 -
>  12 files changed, 61 insertions(+), 64 deletions(-)

Added to misc-next, thanks. Patch 2 was skipped (the rename from
disk_bytenr to logical) to keep the on-disk naming in the structures.