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. Other than that, it looks good, thanks. Reviewed-by: Filipe Manana <fdman...@suse.com> > ret = btrfs_remove_chunk(trans, chunk_offset); > btrfs_end_transaction(trans); > return ret; > -- > 2.30.0 > -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.”
