Re: [PATCH v9 21/41] btrfs: use bio_add_zone_append_page for zoned btrfs

[Josef Bacik <josef@toxicpanda.com>]

On 10/30/20 9:51 AM, Naohiro Aota wrote:
> Zoned device has its own hardware restrictions e.g. max_zone_append_size
> when using REQ_OP_ZONE_APPEND. To follow the restrictions, use
> bio_add_zone_append_page() instead of bio_add_page(). We need target device
> to use bio_add_zone_append_page(), so this commit reads the chunk
> information to memoize the target device to btrfs_io_bio(bio)->device.
> 
> Currently, zoned btrfs only supports SINGLE profile. In the feature,
> btrfs_io_bio can hold extent_map and check the restrictions for all the
> devices the bio will be mapped.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>   fs/btrfs/extent_io.c | 37 ++++++++++++++++++++++++++++++++++---
>   1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 17285048fb5a..764257eb658f 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3032,6 +3032,7 @@ bool btrfs_bio_add_page(struct bio *bio, struct page *page, u64 logical,
>   {
>   	sector_t sector = logical >> SECTOR_SHIFT;
>   	bool contig;
> +	int ret;
>   
>   	if (prev_bio_flags != bio_flags)
>   		return false;
> @@ -3046,7 +3047,19 @@ bool btrfs_bio_add_page(struct bio *bio, struct page *page, u64 logical,
>   	if (btrfs_bio_fits_in_stripe(page, size, bio, bio_flags))
>   		return false;
>   
> -	return bio_add_page(bio, page, size, pg_offset) == size;
> +	if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
> +		struct bio orig_bio;
> +
> +		memset(&orig_bio, 0, sizeof(orig_bio));
> +		bio_copy_dev(&orig_bio, bio);
> +		bio_set_dev(bio, btrfs_io_bio(bio)->device->bdev);
> +		ret = bio_add_zone_append_page(bio, page, size, pg_offset);
> +		bio_copy_dev(bio, &orig_bio);

Why do we need this in the first place, since we're only supporting single right 
now?  latest_bdev should be the same, so this serves no purpose, right?

And if it does, we need to figure out another solution, because right now this 
leaks references to the bio->bi_blkg, each copy inc's the refcount on the blkg 
for that bio so we're going to leak blkg's like crazy here.  Thanks,

Josef