| Message ID | fc988b42d58cf2e6b0ae2030fe0e67033ce27eca.1628242009.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: zoned: allow disabling of zone auto relcaim | expand |
On Fri, Aug 06, 2021 at 06:27:04PM +0900, Johannes Thumshirn wrote: > Automatically reclaiming dirty zones might not always be desired for all > workloads, especially as there are currently still some rough edges with > the relocation code on zoned filesystems. > > Allow disabling zone auto reclaim on a per filesystem basis. > > Cc: Naohiro Aota <naohiro.aota@wdc.com> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > --- > fs/btrfs/free-space-cache.c | 3 ++- > fs/btrfs/sysfs.c | 5 ++++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c > index 8eeb65278ac0..933e9de37802 100644 > --- a/fs/btrfs/free-space-cache.c > +++ b/fs/btrfs/free-space-cache.c > @@ -2567,7 +2567,8 @@ static int __btrfs_add_free_space_zoned(struct btrfs_block_group *block_group, > /* All the region is now unusable. Mark it as unused and reclaim */ > if (block_group->zone_unusable == block_group->length) { > btrfs_mark_bg_unused(block_group); > - } else if (block_group->zone_unusable >= > + } else if (fs_info->bg_reclaim_threshold && > + block_group->zone_unusable >= > div_factor_fine(block_group->length, > fs_info->bg_reclaim_threshold)) { nit: can this race with btrfs_bg_reclaim_threshold_store()'s bg_reclaim_threshold assignment? Then, we can end up doing div_factor_fine(block_group->length, 0)? > btrfs_mark_bg_to_reclaim(block_group); > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index bfe5e27617b0..5f18c3e3d837 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -1001,11 +1001,14 @@ static ssize_t btrfs_bg_reclaim_threshold_store(struct kobject *kobj, > if (ret) > return ret; > > - if (thresh <= 50 || thresh > 100) > + if (thresh != 0 && (thresh <= 50 || thresh > 100)) > return -EINVAL; > > fs_info->bg_reclaim_threshold = thresh; > > + if (thresh == 0) > + btrfs_info(fs_info, "disabling auto reclaim"); > + > return len; > } > BTRFS_ATTR_RW(, bg_reclaim_threshold, btrfs_bg_reclaim_threshold_show, > -- > 2.32.0 >
On Mon, Aug 09, 2021 at 07:50:59AM +0000, Naohiro Aota wrote: > On Fri, Aug 06, 2021 at 06:27:04PM +0900, Johannes Thumshirn wrote: > > Automatically reclaiming dirty zones might not always be desired for all > > workloads, especially as there are currently still some rough edges with > > the relocation code on zoned filesystems. > > > > Allow disabling zone auto reclaim on a per filesystem basis. > > > > Cc: Naohiro Aota <naohiro.aota@wdc.com> > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > > > --- > > fs/btrfs/free-space-cache.c | 3 ++- > > fs/btrfs/sysfs.c | 5 ++++- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c > > index 8eeb65278ac0..933e9de37802 100644 > > --- a/fs/btrfs/free-space-cache.c > > +++ b/fs/btrfs/free-space-cache.c > > @@ -2567,7 +2567,8 @@ static int __btrfs_add_free_space_zoned(struct btrfs_block_group *block_group, > > /* All the region is now unusable. Mark it as unused and reclaim */ > > if (block_group->zone_unusable == block_group->length) { > > btrfs_mark_bg_unused(block_group); > > - } else if (block_group->zone_unusable >= > > + } else if (fs_info->bg_reclaim_threshold && > > + block_group->zone_unusable >= > > div_factor_fine(block_group->length, > > fs_info->bg_reclaim_threshold)) { > > nit: can this race with btrfs_bg_reclaim_threshold_store()'s > bg_reclaim_threshold assignment? Then, we can end up doing > div_factor_fine(block_group->length, 0)? Good point, this should be READ_ONCE and then check if it's 0.
On 09/08/2021 10:09, David Sterba wrote: > On Mon, Aug 09, 2021 at 07:50:59AM +0000, Naohiro Aota wrote: >> On Fri, Aug 06, 2021 at 06:27:04PM +0900, Johannes Thumshirn wrote: >>> Automatically reclaiming dirty zones might not always be desired for all >>> workloads, especially as there are currently still some rough edges with >>> the relocation code on zoned filesystems. >>> >>> Allow disabling zone auto reclaim on a per filesystem basis. >>> >>> Cc: Naohiro Aota <naohiro.aota@wdc.com> >>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> >>> >>> --- >>> fs/btrfs/free-space-cache.c | 3 ++- >>> fs/btrfs/sysfs.c | 5 ++++- >>> 2 files changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c >>> index 8eeb65278ac0..933e9de37802 100644 >>> --- a/fs/btrfs/free-space-cache.c >>> +++ b/fs/btrfs/free-space-cache.c >>> @@ -2567,7 +2567,8 @@ static int __btrfs_add_free_space_zoned(struct btrfs_block_group *block_group, >>> /* All the region is now unusable. Mark it as unused and reclaim */ >>> if (block_group->zone_unusable == block_group->length) { >>> btrfs_mark_bg_unused(block_group); >>> - } else if (block_group->zone_unusable >= >>> + } else if (fs_info->bg_reclaim_threshold && >>> + block_group->zone_unusable >= >>> div_factor_fine(block_group->length, >>> fs_info->bg_reclaim_threshold)) { >> >> nit: can this race with btrfs_bg_reclaim_threshold_store()'s >> bg_reclaim_threshold assignment? Then, we can end up doing >> div_factor_fine(block_group->length, 0)? > > Good point, this should be READ_ONCE and then check if it's 0. > Oh yeah definitively. I'll fix it up ASAP.
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index 8eeb65278ac0..933e9de37802 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -2567,7 +2567,8 @@ static int __btrfs_add_free_space_zoned(struct btrfs_block_group *block_group, /* All the region is now unusable. Mark it as unused and reclaim */ if (block_group->zone_unusable == block_group->length) { btrfs_mark_bg_unused(block_group); - } else if (block_group->zone_unusable >= + } else if (fs_info->bg_reclaim_threshold && + block_group->zone_unusable >= div_factor_fine(block_group->length, fs_info->bg_reclaim_threshold)) { btrfs_mark_bg_to_reclaim(block_group); diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index bfe5e27617b0..5f18c3e3d837 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -1001,11 +1001,14 @@ static ssize_t btrfs_bg_reclaim_threshold_store(struct kobject *kobj, if (ret) return ret; - if (thresh <= 50 || thresh > 100) + if (thresh != 0 && (thresh <= 50 || thresh > 100)) return -EINVAL; fs_info->bg_reclaim_threshold = thresh; + if (thresh == 0) + btrfs_info(fs_info, "disabling auto reclaim"); + return len; } BTRFS_ATTR_RW(, bg_reclaim_threshold, btrfs_bg_reclaim_threshold_show,
Automatically reclaiming dirty zones might not always be desired for all workloads, especially as there are currently still some rough edges with the relocation code on zoned filesystems. Allow disabling zone auto reclaim on a per filesystem basis. Cc: Naohiro Aota <naohiro.aota@wdc.com> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- fs/btrfs/free-space-cache.c | 3 ++- fs/btrfs/sysfs.c | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-)