[PATCH] btrfs-progs: default to SINGLE profile on zoned devices

[PATCH] btrfs-progs: default to SINGLE profile on zoned devices

[Johannes Thumshirn]

On zoned devices we're currently not supporting any other block group
profile than the SINGLE profile, so pick it as default value otherwise a
user would have to specify it manually at mkfs time for rotational zoned
devices.

Reported-by: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 mkfs/common.h | 2 ++
 mkfs/main.c   | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/mkfs/common.h b/mkfs/common.h
index ea87c3cabccf..cd98c3717235 100644
--- a/mkfs/common.h
+++ b/mkfs/common.h
@@ -34,9 +34,11 @@
 #define BTRFS_MKFS_DEFAULT_DATA_ONE_DEVICE	0	/* SINGLE */
 #define BTRFS_MKFS_DEFAULT_META_ONE_DEVICE	BTRFS_BLOCK_GROUP_DUP
 #define BTRFS_MKFS_DEFAULT_META_ONE_DEVICE_SSD	0	/* SINGLE */
+#define BTRFS_MKFS_DEFAULT_META_ONE_DEVICE_ZONED 0	/* SINGLE */
 
 #define BTRFS_MKFS_DEFAULT_DATA_MULTI_DEVICE	0	/* SINGLE */
 #define BTRFS_MKFS_DEFAULT_META_MULTI_DEVICE	BTRFS_BLOCK_GROUP_RAID1
+#define BTRFS_MKFS_DEFAULT_META_MULTI_DEVICE_ZONED 0	/* SINGLE */
 
 struct btrfs_trans_handle;
 struct btrfs_root;
diff --git a/mkfs/main.c b/mkfs/main.c
index d2322fafc862..fb68136a8389 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1149,10 +1149,15 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 				"force metadata duplication.\n");
 
 			if (dev_cnt > 1) {
-				tmp = BTRFS_MKFS_DEFAULT_META_MULTI_DEVICE;
+				if (zoned)
+					tmp = BTRFS_MKFS_DEFAULT_META_MULTI_DEVICE_ZONED;
+				else
+					tmp = BTRFS_MKFS_DEFAULT_META_MULTI_DEVICE;
 			} else {
 				if (ssd)
 					tmp = BTRFS_MKFS_DEFAULT_META_ONE_DEVICE_SSD;
+				else if (zoned)
+					tmp = BTRFS_MKFS_DEFAULT_META_ONE_DEVICE_ZONED;
 				else
 					tmp = BTRFS_MKFS_DEFAULT_META_ONE_DEVICE;
 			}
-- 
2.31.1

Re: [PATCH] btrfs-progs: default to SINGLE profile on zoned devices

[Nikolay Borisov]

On 6.07.21 г. 12:19, Johannes Thumshirn wrote:
> On zoned devices we're currently not supporting any other block group
> profile than the SINGLE profile, so pick it as default value otherwise a
> user would have to specify it manually at mkfs time for rotational zoned
> devices.
> 
> Reported-by: Damien Le Moal <damien.lemoal@wdc.com>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

Re: [PATCH] btrfs-progs: default to SINGLE profile on zoned devices

[David Sterba]

On Tue, Jul 06, 2021 at 06:19:22PM +0900, Johannes Thumshirn wrote:
> On zoned devices we're currently not supporting any other block group
> profile than the SINGLE profile, so pick it as default value otherwise a
> user would have to specify it manually at mkfs time for rotational zoned
> devices.

Yes this is annoying but careful with setting defaults, it's hard to
change them. And in case of zoned devices it will be possible to set
something else in the future so defaulting to single/single needs to be
justified in another way than "currently we don't support anything
else".

The SSD fallback to single is not showing as useful and there's ongoing
work to make it default to dup for metadata again. For consistency I'd
rather have simple logic for selecting defaults and give hints
eventually instead of checking random things in the system and then
selectin on behalf of the user. Unfortunatelly it's not that easy as
there are conflicting valid interests and we don't have defaults that
fits all scenarios.

Re: [PATCH] btrfs-progs: default to SINGLE profile on zoned devices

[Johannes Thumshirn]

On 07/07/2021 16:53, David Sterba wrote:
> On Tue, Jul 06, 2021 at 06:19:22PM +0900, Johannes Thumshirn wrote:
>> On zoned devices we're currently not supporting any other block group
>> profile than the SINGLE profile, so pick it as default value otherwise a
>> user would have to specify it manually at mkfs time for rotational zoned
>> devices.
> 
> Yes this is annoying but careful with setting defaults, it's hard to
> change them. And in case of zoned devices it will be possible to set
> something else in the future so defaulting to single/single needs to be
> justified in another way than "currently we don't support anything
> else".
> 
> The SSD fallback to single is not showing as useful and there's ongoing
> work to make it default to dup for metadata again. For consistency I'd
> rather have simple logic for selecting defaults and give hints
> eventually instead of checking random things in the system and then
> selectin on behalf of the user. Unfortunatelly it's not that easy as
> there are conflicting valid interests and we don't have defaults that
> fits all scenarios.
> 

Agreed, but without this patch mkfs with default parameters on a rotational
zoned device will fail with:

johannes@redsun60:btrfs-progs(master)$ sudo ./mkfs.btrfs /dev/sda
btrfs-progs v5.12.1 
See http://btrfs.wiki.kernel.org for more information.

Zoned: /dev/sda: host-managed device detected, setting zoned feature
ERROR: cannot use RAID/DUP profile in zoned mode

So defaulting to not creating a filesystem won't work either.

We could improve the error message hinting the user to specify 
"-m single -d single" on mkfs but that feels more hacky than setting 
the defaults to something working.

Re: [PATCH] btrfs-progs: default to SINGLE profile on zoned devices

[Johannes Thumshirn]

On 07/07/2021 17:02, Johannes Thumshirn wrote:
> On 07/07/2021 16:53, David Sterba wrote:
>> On Tue, Jul 06, 2021 at 06:19:22PM +0900, Johannes Thumshirn wrote:
>>> On zoned devices we're currently not supporting any other block group
>>> profile than the SINGLE profile, so pick it as default value otherwise a
>>> user would have to specify it manually at mkfs time for rotational zoned
>>> devices.
>>
>> Yes this is annoying but careful with setting defaults, it's hard to
>> change them. And in case of zoned devices it will be possible to set
>> something else in the future so defaulting to single/single needs to be
>> justified in another way than "currently we don't support anything
>> else".
>>
>> The SSD fallback to single is not showing as useful and there's ongoing
>> work to make it default to dup for metadata again. For consistency I'd
>> rather have simple logic for selecting defaults and give hints
>> eventually instead of checking random things in the system and then
>> selectin on behalf of the user. Unfortunatelly it's not that easy as
>> there are conflicting valid interests and we don't have defaults that
>> fits all scenarios.
>>
> 
> Agreed, but without this patch mkfs with default parameters on a rotational
> zoned device will fail with:
> 
> johannes@redsun60:btrfs-progs(master)$ sudo ./mkfs.btrfs /dev/sda
> btrfs-progs v5.12.1 
> See http://btrfs.wiki.kernel.org for more information.
> 
> Zoned: /dev/sda: host-managed device detected, setting zoned feature
> ERROR: cannot use RAID/DUP profile in zoned mode
> 
> So defaulting to not creating a filesystem won't work either.
> 
> We could improve the error message hinting the user to specify 
> "-m single -d single" on mkfs but that feels more hacky than setting 
> the defaults to something working.
> 

So what's the way to go here? The current default won't create a file
system so it's unusable.

Re: [PATCH] btrfs-progs: default to SINGLE profile on zoned devices

[David Sterba]

On Mon, Jul 12, 2021 at 07:16:36PM +0000, Johannes Thumshirn wrote:
> On 07/07/2021 17:02, Johannes Thumshirn wrote:
> > On 07/07/2021 16:53, David Sterba wrote:
> >> On Tue, Jul 06, 2021 at 06:19:22PM +0900, Johannes Thumshirn wrote:
> >>> On zoned devices we're currently not supporting any other block group
> >>> profile than the SINGLE profile, so pick it as default value otherwise a
> >>> user would have to specify it manually at mkfs time for rotational zoned
> >>> devices.
> >>
> >> Yes this is annoying but careful with setting defaults, it's hard to
> >> change them. And in case of zoned devices it will be possible to set
> >> something else in the future so defaulting to single/single needs to be
> >> justified in another way than "currently we don't support anything
> >> else".
> >>
> >> The SSD fallback to single is not showing as useful and there's ongoing
> >> work to make it default to dup for metadata again. For consistency I'd
> >> rather have simple logic for selecting defaults and give hints
> >> eventually instead of checking random things in the system and then
> >> selectin on behalf of the user. Unfortunatelly it's not that easy as
> >> there are conflicting valid interests and we don't have defaults that
> >> fits all scenarios.
> >>
> > 
> > Agreed, but without this patch mkfs with default parameters on a rotational
> > zoned device will fail with:
> > 
> > johannes@redsun60:btrfs-progs(master)$ sudo ./mkfs.btrfs /dev/sda
> > btrfs-progs v5.12.1 
> > See http://btrfs.wiki.kernel.org for more information.
> > 
> > Zoned: /dev/sda: host-managed device detected, setting zoned feature
> > ERROR: cannot use RAID/DUP profile in zoned mode
> > 
> > So defaulting to not creating a filesystem won't work either.
> > 
> > We could improve the error message hinting the user to specify 
> > "-m single -d single" on mkfs but that feels more hacky than setting 
> > the defaults to something working.
> > 
> 
> So what's the way to go here? The current default won't create a file
> system so it's unusable.

I've updated the message to ask for -d single -m single.