| Message ID | d09710513b6f8ad50973d894129b1dd441f2ab83.1633950641.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: zoned: use greedy gc for auto reclaim | expand |
On Mon, Oct 11, 2021 at 08:11:32PM +0900, Johannes Thumshirn wrote: > Currently auto reclaim of unusable zones reclaims the block-groups in the > order they have been added to the reclaim list. > > Change this to a greedy algorithm by sorting the list so we have the > block-groups with the least amount of valid bytes reclaimed first. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > --- > Changes since RFC: > - Updated the patch description > - Don't sort the list under the spin_lock (David) > --- > fs/btrfs/block-group.c | 29 +++++++++++++++++++++++++---- > 1 file changed, 25 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c > index 46fdef7bbe20..930c07cdad81 100644 > --- a/fs/btrfs/block-group.c > +++ b/fs/btrfs/block-group.c > @@ -1,5 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > > +#include <linux/list_sort.h> > + > #include "misc.h" > #include "ctree.h" > #include "block-group.h" > @@ -1486,6 +1488,21 @@ void btrfs_mark_bg_unused(struct btrfs_block_group *bg) > spin_unlock(&fs_info->unused_bgs_lock); > } > > +/* > + * We want block groups with a low number of used bytes to be in the beginning > + * of the list, so they will get reclaimed first. > + */ > +static int reclaim_bgs_cmp(void *unused, const struct list_head *a, > + const struct list_head *b) > +{ > + const struct btrfs_block_group *bg1, *bg2; > + > + bg1 = list_entry(a, struct btrfs_block_group, bg_list); > + bg2 = list_entry(b, struct btrfs_block_group, bg_list); > + > + return bg1->used - bg2->used; It returns int but compares two u64, this should be either a if chaing comparing all the possibilities or properly cast to s64.
On 11/10/2021 16:52, David Sterba wrote: >> +static int reclaim_bgs_cmp(void *unused, const struct list_head *a, >> + const struct list_head *b) >> +{ >> + const struct btrfs_block_group *bg1, *bg2; >> + >> + bg1 = list_entry(a, struct btrfs_block_group, bg_list); >> + bg2 = list_entry(b, struct btrfs_block_group, bg_list); >> + >> + return bg1->used - bg2->used; > > It returns int but compares two u64, this should be either a if chaing > comparing all the possibilities or properly cast to s64. > Looking at the documentation of list_sort() this can also be used as a boolean, I'll do that then.
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 46fdef7bbe20..930c07cdad81 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 +#include <linux/list_sort.h> + #include "misc.h" #include "ctree.h" #include "block-group.h" @@ -1486,6 +1488,21 @@ void btrfs_mark_bg_unused(struct btrfs_block_group *bg) spin_unlock(&fs_info->unused_bgs_lock); } +/* + * We want block groups with a low number of used bytes to be in the beginning + * of the list, so they will get reclaimed first. + */ +static int reclaim_bgs_cmp(void *unused, const struct list_head *a, + const struct list_head *b) +{ + const struct btrfs_block_group *bg1, *bg2; + + bg1 = list_entry(a, struct btrfs_block_group, bg_list); + bg2 = list_entry(b, struct btrfs_block_group, bg_list); + + return bg1->used - bg2->used; +} + void btrfs_reclaim_bgs_work(struct work_struct *work) { struct btrfs_fs_info *fs_info = @@ -1493,6 +1510,7 @@ void btrfs_reclaim_bgs_work(struct work_struct *work) struct btrfs_block_group *bg; struct btrfs_space_info *space_info; LIST_HEAD(again_list); + LIST_HEAD(reclaim_list); if (!test_bit(BTRFS_FS_OPEN, &fs_info->flags)) return; @@ -1510,17 +1528,20 @@ void btrfs_reclaim_bgs_work(struct work_struct *work) } spin_lock(&fs_info->unused_bgs_lock); - while (!list_empty(&fs_info->reclaim_bgs)) { + list_splice_init(&fs_info->reclaim_bgs, &reclaim_list); + spin_unlock(&fs_info->unused_bgs_lock); + + list_sort(NULL, &reclaim_list, reclaim_bgs_cmp); + while (!list_empty(&reclaim_list)) { u64 zone_unusable; int ret = 0; - bg = list_first_entry(&fs_info->reclaim_bgs, + bg = list_first_entry(&reclaim_list, struct btrfs_block_group, bg_list); list_del_init(&bg->bg_list); space_info = bg->space_info; - spin_unlock(&fs_info->unused_bgs_lock); /* Don't race with allocators so take the groups_sem */ down_write(&space_info->groups_sem); @@ -1568,12 +1589,12 @@ void btrfs_reclaim_bgs_work(struct work_struct *work) bg->start); next: - spin_lock(&fs_info->unused_bgs_lock); if (ret == -EAGAIN && list_empty(&bg->bg_list)) list_add_tail(&bg->bg_list, &again_list); else btrfs_put_block_group(bg); } + spin_lock(&fs_info->unused_bgs_lock); list_splice_tail(&again_list, &fs_info->reclaim_bgs); spin_unlock(&fs_info->unused_bgs_lock); mutex_unlock(&fs_info->reclaim_bgs_lock);
Currently auto reclaim of unusable zones reclaims the block-groups in the order they have been added to the reclaim list. Change this to a greedy algorithm by sorting the list so we have the block-groups with the least amount of valid bytes reclaimed first. Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- Changes since RFC: - Updated the patch description - Don't sort the list under the spin_lock (David) --- fs/btrfs/block-group.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)