linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btrfs: zoned: allow disabling of zone auto relcaim
@ 2021-08-06  9:27 Johannes Thumshirn
  2021-08-09  7:50 ` Naohiro Aota
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Thumshirn @ 2021-08-06  9:27 UTC (permalink / raw)
  To: David Sterba; +Cc: Johannes Thumshirn, linux-btrfs, Naohiro Aota

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)) {
 		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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: zoned: allow disabling of zone auto relcaim
  2021-08-06  9:27 [PATCH] btrfs: zoned: allow disabling of zone auto relcaim Johannes Thumshirn
@ 2021-08-09  7:50 ` Naohiro Aota
  2021-08-09  8:06   ` David Sterba
  0 siblings, 1 reply; 4+ messages in thread
From: Naohiro Aota @ 2021-08-09  7:50 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: David Sterba, linux-btrfs

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: zoned: allow disabling of zone auto relcaim
  2021-08-09  7:50 ` Naohiro Aota
@ 2021-08-09  8:06   ` David Sterba
  2021-08-09  8:10     ` Johannes Thumshirn
  0 siblings, 1 reply; 4+ messages in thread
From: David Sterba @ 2021-08-09  8:06 UTC (permalink / raw)
  To: Naohiro Aota; +Cc: Johannes Thumshirn, linux-btrfs

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: zoned: allow disabling of zone auto relcaim
  2021-08-09  8:06   ` David Sterba
@ 2021-08-09  8:10     ` Johannes Thumshirn
  0 siblings, 0 replies; 4+ messages in thread
From: Johannes Thumshirn @ 2021-08-09  8:10 UTC (permalink / raw)
  To: dsterba, Naohiro Aota; +Cc: linux-btrfs

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-09  8:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06  9:27 [PATCH] btrfs: zoned: allow disabling of zone auto relcaim Johannes Thumshirn
2021-08-09  7:50 ` Naohiro Aota
2021-08-09  8:06   ` David Sterba
2021-08-09  8:10     ` Johannes Thumshirn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).