| Message ID | 20210706091922.38650-1-johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: default to SINGLE profile on zoned devices | expand |
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>
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.
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.
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.
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.
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; }
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(-)