[PATCH] common/rc: don't clear superblock for zoned scratch pools

[PATCH] common/rc: don't clear superblock for zoned scratch pools

[Johannes Thumshirn]

_require_scratch_dev_pool() zeros the first 100 sectors of each device to
clear eventual remains of older filesystems.

On zoned devices this creates all sorts of problems, so just skip the
clearing there.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 common/rc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index 654730b21ead..d763501be2b2 100644
--- a/common/rc
+++ b/common/rc
@@ -3461,7 +3461,9 @@ _require_scratch_dev_pool()
 		fi
 		# to help better debug when something fails, we remove
 		# traces of previous btrfs FS on the dev.
-		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
+		if [ "`_zone_type "$i"`" = "none" ]; then
+			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
+		fi
 	done
 }
 
-- 
2.39.1

Re: [PATCH] common/rc: don't clear superblock for zoned scratch pools

[Filipe Manana]

On Thu, Feb 23, 2023 at 3:56 PM Johannes Thumshirn
<johannes.thumshirn@wdc.com> wrote:
>
> _require_scratch_dev_pool() zeros the first 100 sectors of each device to
> clear eventual remains of older filesystems.
>
> On zoned devices this creates all sorts of problems, so just skip the
> clearing there.

What kind of problems? Can you give 1 or 2 examples at least?
And it would be nice to comment in the code why we don't zero the
sectors for zoned devices.

Thanks.

>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  common/rc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index 654730b21ead..d763501be2b2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3461,7 +3461,9 @@ _require_scratch_dev_pool()
>                 fi
>                 # to help better debug when something fails, we remove
>                 # traces of previous btrfs FS on the dev.
> -               dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +               if [ "`_zone_type "$i"`" = "none" ]; then
> +                       dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +               fi
>         done
>  }
>
> --
> 2.39.1
>

Re: [PATCH] common/rc: don't clear superblock for zoned scratch pools

[Naohiro Aota]

On Thu, Feb 23, 2023 at 07:40:35AM -0800, Johannes Thumshirn wrote:
> _require_scratch_dev_pool() zeros the first 100 sectors of each device to
> clear eventual remains of older filesystems.
> 
> On zoned devices this creates all sorts of problems, so just skip the
> clearing there.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  common/rc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 654730b21ead..d763501be2b2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3461,7 +3461,9 @@ _require_scratch_dev_pool()
>  		fi
>  		# to help better debug when something fails, we remove
>  		# traces of previous btrfs FS on the dev.
> -		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		if [ "`_zone_type "$i"`" = "none" ]; then
> +			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		fi

How about resetting the first two zones on the zoned device case?

>  	done
>  }
>  
> -- 
> 2.39.1
> 

Re: [PATCH] common/rc: don't clear superblock for zoned scratch pools

[Johannes Thumshirn]

On 24.02.23 08:26, Naohiro Aota wrote:
> On Thu, Feb 23, 2023 at 07:40:35AM -0800, Johannes Thumshirn wrote:
>> _require_scratch_dev_pool() zeros the first 100 sectors of each device to
>> clear eventual remains of older filesystems.
>>
>> On zoned devices this creates all sorts of problems, so just skip the
>> clearing there.
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> ---
>>  common/rc | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/rc b/common/rc
>> index 654730b21ead..d763501be2b2 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -3461,7 +3461,9 @@ _require_scratch_dev_pool()
>>  		fi
>>  		# to help better debug when something fails, we remove
>>  		# traces of previous btrfs FS on the dev.
>> -		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
>> +		if [ "`_zone_type "$i"`" = "none" ]; then
>> +			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
>> +		fi
> 
> How about resetting the first two zones on the zoned device case?
> 

yep that could work.

Re: [PATCH] common/rc: don't clear superblock for zoned scratch pools

[Johannes Thumshirn]

On 23.02.23 18:02, Filipe Manana wrote:
>> On zoned devices this creates all sorts of problems, so just skip the
>> clearing there.
> What kind of problems? Can you give 1 or 2 examples at least?
> And it would be nice to comment in the code why we don't zero the
> sectors for zoned devices.

It will lead to unaligned write errors consequently failing subsequent
mkfs and failing every tests that has _require_scratch_dev_pool.

I'll add it to v2.

Re: [PATCH] common/rc: don't clear superblock for zoned scratch pools

[Zorro Lang]

On Thu, Feb 23, 2023 at 07:40:35AM -0800, Johannes Thumshirn wrote:
> _require_scratch_dev_pool() zeros the first 100 sectors of each device to
> clear eventual remains of older filesystems.
> 
> On zoned devices this creates all sorts of problems, so just skip the
> clearing there.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  common/rc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 654730b21ead..d763501be2b2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3461,7 +3461,9 @@ _require_scratch_dev_pool()
>  		fi
>  		# to help better debug when something fails, we remove
>  		# traces of previous btrfs FS on the dev.
> -		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		if [ "`_zone_type "$i"`" = "none" ]; then
> +			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		fi

Better to add some comments to explain why we need to check
[ "`_zone_type "$i"`" = "none" ] at here, even if only copy
your git commit log at here.

>  	done
>  }
>  
> -- 
> 2.39.1
>