On 16/04/2021 11:12, Filipe Manana wrote: > On Thu, Apr 15, 2021 at 3:00 PM Johannes Thumshirn > <johannes.thumsh...@wdc.com> 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/459e2932c48e12e883dcfd3dda828d9da251d5b5.1617962110.git.johannes.thumsh...@wdc.com >> Signed-off-by: Johannes Thumshirn <johannes.thumsh...@wdc.com> >> --- >> fs/btrfs/volumes.c | 21 +++++++++++++++++---- >> 1 file changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 6d9b2369f17a..b1bab75ec12a 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,24 @@ 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); >> >> + /* >> + * Step two, delete the device extents and the chunk tree entries. >> + * >> + * 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)) { >> @@ -3140,10 +3157,6 @@ static int btrfs_relocate_chunk(struct btrfs_fs_info >> *fs_info, u64 chunk_offset) >> return ret; >> } >> >> - /* >> - * step two, delete the device extents and the >> - * chunk tree entries >> - */ > > This hunk seems unrelated and unintentional. > Not that the comment adds any value, but if the removal was > intentional, I would suggest a separate patch for that.
It's moved upwards + /* + * Step two, delete the device extents and the chunk tree entries. + * + * 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. + */ 'cause technically the "delete extents" step starts with the discard now. But I can leave it where it was. > Other than that, it looks good, thanks. > > Reviewed-by: Filipe Manana <fdman...@suse.com> > Thanks
