| Message ID | 20230223154035.296702-1-johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | common/rc: don't clear superblock for zoned scratch pools | expand |
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 >
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 >
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.
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.
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 >
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 }
_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(-)