[v5,1/3] btrfs: zoned: reset zones of relocated block groups

Commit Message

Johannes Thumshirn April 19, 2021, 7:41 a.m. UTC

When relocating a block group the freed up space is not discarded in one
big block, but each extent is discarded on it's own with -odisard=sync.

For a zoned filesystem we need to discard the whole block group at once,
so btrfs_discard_extent() will translate the discard into a
REQ_OP_ZONE_RESET operation, which then resets the device's zone.

Link: https://lore.kernel.org/linux-btrfs/459e2932c4[e-mail]
Signed-off-by: Johannes Thumshirn <[e-mail]>
Reviewed-by: Josef Bacik <[e-mail]>
Reviewed-by: Filipe Manana <[e-mail]>
---
 fs/btrfs/volumes.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

David Sterba April 19, 2021, 5:10 p.m. UTC | #1

On Mon, Apr 19, 2021 at 04:41:00PM +0900, Johannes Thumshirn wrote:
> When relocating a block group the freed up space is not discarded in one
> big block, but each extent is discarded on it's own with -odisard=sync.
> 
> For a zoned filesystem we need to discard the whole block group at once,
> so btrfs_discard_extent() will translate the discard into a
> REQ_OP_ZONE_RESET operation, which then resets the device's zone.
> 
> Link: https://lore.kernel.org/linux-btrfs/459e2932c4[e-mail]

The link points to the same patch in v3, is there something missing in
this patch that should be added to the changelog?

Johannes Thumshirn April 20, 2021, 7:40 a.m. UTC | #2

On 19/04/2021 19:13, David Sterba wrote:
> On Mon, Apr 19, 2021 at 04:41:00PM +0900, Johannes Thumshirn wrote:
>> When relocating a block group the freed up space is not discarded in one
>> big block, but each extent is discarded on it's own with -odisard=sync.
>>
>> For a zoned filesystem we need to discard the whole block group at once,
>> so btrfs_discard_extent() will translate the discard into a
>> REQ_OP_ZONE_RESET operation, which then resets the device's zone.
>>
>> Link: https://lore.kernel.org/linux-btrfs/459e2932c4[e-mail]
> 
> The link points to the same patch in v3, is there something missing in
> this patch that should be added to the changelog?
> 

I included this link so one can look up the discussion between Filipe and
me that lead to this decision. But I guess this is not really relevant.

David Sterba April 20, 2021, 6:14 p.m. UTC | #3

On Tue, Apr 20, 2021 at 07:40:37AM +0000, Johannes Thumshirn wrote:
> On 19/04/2021 19:13, David Sterba wrote:
> > On Mon, Apr 19, 2021 at 04:41:00PM +0900, Johannes Thumshirn wrote:
> >> When relocating a block group the freed up space is not discarded in one
> >> big block, but each extent is discarded on it's own with -odisard=sync.
> >>
> >> For a zoned filesystem we need to discard the whole block group at once,
> >> so btrfs_discard_extent() will translate the discard into a
> >> REQ_OP_ZONE_RESET operation, which then resets the device's zone.
> >>
> >> Link: https://lore.kernel.org/linux-btrfs/459e2932c4[e-mail]
> > 
> > The link points to the same patch in v3, is there something missing in
> > this patch that should be added to the changelog?
> > 
> 
> I included this link so one can look up the discussion between Filipe and
> me that lead to this decision. But I guess this is not really relevant. 

So what'd recommend and expect is to put a summary of the discussion to
the changelog, because the discussions can take many mails and the
contents is scattered, and link the first relevant mail in case the full
story could be interesting. Eventually the link could be in text with
some hint what to look for in the mail.

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6d9b2369f17a..124c17ec53e9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3103,6 +3103,7 @@  static int btrfs_relocate_chunk(struct btrfs_fs_info *fs_info, u64 chunk_offset)
 	struct btrfs_root *root = fs_info->chunk_root;
 	struct btrfs_trans_handle *trans;
 	struct btrfs_block_group *block_group;
+	u64 length;
 	int ret;
 
 	/*
@@ -3130,8 +3131,22 @@  static int btrfs_relocate_chunk(struct btrfs_fs_info *fs_info, u64 chunk_offset)
 	if (!block_group)
 		return -ENOENT;
 	btrfs_discard_cancel_work(&fs_info->discard_ctl, block_group);
+	length = block_group->length;
 	btrfs_put_block_group(block_group);
 
+	/*
+	 * On a zoned file system, discard the whole block group, this will
+	 * trigger a REQ_OP_ZONE_RESET operation on the device zone. If
+	 * resetting the zone fails, don't treat it as a fatal problem from the
+	 * filesystem's point of view.
+	 */
+	if (btrfs_is_zoned(fs_info)) {
+		ret = btrfs_discard_extent(fs_info, chunk_offset, length, NULL);
+		if (ret)
+			btrfs_info(fs_info, "failed to reset zone %llu",
+				   chunk_offset);
+	}
+
 	trans = btrfs_start_trans_remove_block_group(root->fs_info,
 						     chunk_offset);
 	if (IS_ERR(trans)) {