Re: [PATCH v7 00/39] btrfs: zoned block device support

From: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
To: Naohiro Aota <Naohiro.Aota@wdc.com>
Cc: "dsterba@suse.cz" <dsterba@suse.cz>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	David Sterba <dsterba@suse.com>, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	Hannes Reinecke <hare@suse.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v7 00/39] btrfs: zoned block device support
Date: Thu, 17 Sep 2020 07:14:05 +0000	[thread overview]
Message-ID: <SN4PR0401MB3598C5D858920149B4BA13249B3E0@SN4PR0401MB3598.namprd04.prod.outlook.com> (raw)
In-Reply-To: 20200917054033.homtvyj3iffrjile@naota.dhcp.fujisawa.hgst.com

On 17/09/2020 07:40, Naohiro Aota wrote:
> Thank you for fixing this.

Well it was you who had the idea, I just sent it.

>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index bda4e02b5eab..311956697682 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -2753,10 +2753,6 @@ static void end_bio_extent_writepage(struct bio *bio)
>>        u64 end;
>>        struct bvec_iter_all iter_all;
>>
>> -       btrfs_record_physical_zoned(bio_iovec(bio).bv_page->mapping->host,
>> -                                   page_offset(bio_iovec(bio).bv_page) + bio_iovec(bio).bv_offset,
>> -                                   bio);
>> -
>>        ASSERT(!bio_flagged(bio, BIO_CLONED));
>>        bio_for_each_segment_all(bvec, bio, iter_all) {
>>                struct page *page = bvec->bv_page;
>> @@ -2782,6 +2778,7 @@ static void end_bio_extent_writepage(struct bio *bio)
>>                start = page_offset(page);
>>                end = start + bvec->bv_offset + bvec->bv_len - 1;
>>
>> +               btrfs_record_physical_zoned(inode, start, bio);
> We need to record the physical address only once per an ordered extent.
> So, this should be like:
> 

Right, this would save us a lot of unneeded function calls for the non-zoned
version of btrfs as well.

> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index c21d1dbe314e..0bbe6e52ea0d 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2748,6 +2748,7 @@ static void end_bio_extent_writepage(struct bio *bio)
>          u64 start;
>          u64 end;
>          struct bvec_iter_all iter_all;
> +       bool first_bvec = true;
> 
>          ASSERT(!bio_flagged(bio, BIO_CLONED));
>          bio_for_each_segment_all(bvec, bio, iter_all) {
> @@ -2774,6 +2775,11 @@ static void end_bio_extent_writepage(struct bio *bio)
>                  start = page_offset(page);
>                  end = start + bvec->bv_offset + bvec->bv_len - 1;
> 
> +               if (first_bvec) {
> +                       btrfs_record_physical_zoned(inode, start, bio);
> +                       first_bvec = false;
> +               }
> +
>                  end_extent_writepage(page, error, start, end);
>                  end_page_writeback(page);
>          }
> 
> 
>>                end_extent_writepage(page, error, start, end);
>>                end_page_writeback(page);
>>        }
>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>> index 576f8e333f16..6fdb21029ea9 100644
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> @@ -1086,8 +1086,7 @@ void btrfs_record_physical_zoned(struct inode *inode, u64 file_offset,
>> {
>>        struct btrfs_ordered_extent *ordered;
>>        struct bio_vec bvec = bio_iovec(bio);
>> -       u64 physical = ((u64)bio->bi_iter.bi_sector << SECTOR_SHIFT) +
>> -               bvec.bv_offset;
>> +       u64 physical = (u64)bio->bi_iter.bi_sector << SECTOR_SHIFT;
>>
>>        if (bio_op(bio) != REQ_OP_ZONE_APPEND)
>>                return;
>>