* [PATCH] btrfs: remove skinny extent verbose message
@ 2022-06-23 8:08 Nikolay Borisov
2022-06-23 8:22 ` Qu Wenruo
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Nikolay Borisov @ 2022-06-23 8:08 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Skinny extents have been a default mkfs feature since version 3.18 i
(introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
make skinny-metadata default") ). It really doesn't bring any value to
users to simply remove it.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/disk-io.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8c34d08e3c64..0af4c03279df 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3501,9 +3501,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
else if (fs_info->compress_type == BTRFS_COMPRESS_ZSTD)
features |= BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD;
- if (features & BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA)
- btrfs_info(fs_info, "has skinny extents");
-
/*
* Flag our filesystem as having big metadata blocks if they are bigger
* than the page size.
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] btrfs: remove skinny extent verbose message
2022-06-23 8:08 [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
@ 2022-06-23 8:22 ` Qu Wenruo
2022-06-23 8:30 ` Nikolay Borisov
2022-06-23 14:19 ` David Sterba
2022-06-23 9:30 ` [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Qu Wenruo @ 2022-06-23 8:22 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2022/6/23 16:08, Nikolay Borisov wrote:
> Skinny extents have been a default mkfs feature since version 3.18 i
> (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
> make skinny-metadata default") ). It really doesn't bring any value to
> users to simply remove it.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Looks fine to me.
But this means we need to define the level of (in)compat flags we want
to show in the dmesg.
By default, we have the following lines:
BTRFS info (device loop0): flagging fs with big metadata feature
BTRFS info (device loop0): using free space tree
BTRFS info (device loop0): has skinny extents
BTRFS info (device loop0): enabling ssd optimizations
BTRFS info (device loop0): checking UUID tree
For "big metadata" it's even less meaningful, and it doesn't even have
sysfs entry for it.
For "free space tree" it may be helpful, but if one is really concerning
about the cache version we're using, it's better to go sysfs other than
checking the kernel dmesg.
For "SSD", it's a good thing to output.
For "UUID" tree, it's definitely not useful, even for developers.
For skinny metadata it's the one you're cleaning.
So I guess you can clean up more unnecessary mount messages then?
Thanks,
Qu
> ---
> fs/btrfs/disk-io.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 8c34d08e3c64..0af4c03279df 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3501,9 +3501,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> else if (fs_info->compress_type == BTRFS_COMPRESS_ZSTD)
> features |= BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD;
>
> - if (features & BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA)
> - btrfs_info(fs_info, "has skinny extents");
> -
> /*
> * Flag our filesystem as having big metadata blocks if they are bigger
> * than the page size.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] btrfs: remove skinny extent verbose message
2022-06-23 8:22 ` Qu Wenruo
@ 2022-06-23 8:30 ` Nikolay Borisov
2022-06-23 14:19 ` David Sterba
1 sibling, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2022-06-23 8:30 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 23.06.22 г. 11:22 ч., Qu Wenruo wrote:
>
>
> On 2022/6/23 16:08, Nikolay Borisov wrote:
>> Skinny extents have been a default mkfs feature since version 3.18 i
>> (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
>> make skinny-metadata default") ). It really doesn't bring any value to
>> users to simply remove it.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>
> Looks fine to me.
>
> But this means we need to define the level of (in)compat flags we want
> to show in the dmesg.
>
> By default, we have the following lines:
>
> BTRFS info (device loop0): flagging fs with big metadata feature
> BTRFS info (device loop0): using free space tree
> BTRFS info (device loop0): has skinny extents
> BTRFS info (device loop0): enabling ssd optimizations
> BTRFS info (device loop0): checking UUID tree
>
> For "big metadata" it's even less meaningful, and it doesn't even have
> sysfs entry for it.
Already sent a patch for this one.
>
> For "free space tree" it may be helpful, but if one is really concerning
> about the cache version we're using, it's better to go sysfs other than
> checking the kernel dmesg.
>
> For "SSD", it's a good thing to output.
>
> For "UUID" tree, it's definitely not useful, even for developers.
This is predicated on whether we need to check the UUID tree not on
whether a flag is set, but I agree it could be removed.
>
> For skinny metadata it's the one you're cleaning.
>
> So I guess you can clean up more unnecessary mount messages then?
Yes
>
> Thanks,
> Qu
>> ---
>> fs/btrfs/disk-io.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 8c34d08e3c64..0af4c03279df 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3501,9 +3501,6 @@ int __cold open_ctree(struct super_block *sb,
>> struct btrfs_fs_devices *fs_device
>> else if (fs_info->compress_type == BTRFS_COMPRESS_ZSTD)
>> features |= BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD;
>>
>> - if (features & BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA)
>> - btrfs_info(fs_info, "has skinny extents");
>> -
>> /*
>> * Flag our filesystem as having big metadata blocks if they are
>> bigger
>> * than the page size.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] btrfs: remove skinny extent verbose message
2022-06-23 8:08 [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
2022-06-23 8:22 ` Qu Wenruo
@ 2022-06-23 9:30 ` Nikolay Borisov
2022-06-23 9:52 ` Qu Wenruo
2022-06-24 16:21 ` David Sterba
3 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2022-06-23 9:30 UTC (permalink / raw)
To: linux-btrfs
On 23.06.22 г. 11:08 ч., Nikolay Borisov wrote:
> Skinny extents have been a default mkfs feature since version 3.18 i
> (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
> make skinny-metadata default") ). It really doesn't bring any value to
> users to simply remove it.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Disregard this, I see you've sent a patch demoting it to debug level.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] btrfs: remove skinny extent verbose message
2022-06-23 8:08 [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
2022-06-23 8:22 ` Qu Wenruo
2022-06-23 9:30 ` [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
@ 2022-06-23 9:52 ` Qu Wenruo
2022-06-24 16:21 ` David Sterba
3 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2022-06-23 9:52 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2022/6/23 16:08, Nikolay Borisov wrote:
> Skinny extents have been a default mkfs feature since version 3.18 i
> (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
> make skinny-metadata default") ). It really doesn't bring any value to
> users to simply remove it.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Thanks,
Qu
> ---
> fs/btrfs/disk-io.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 8c34d08e3c64..0af4c03279df 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3501,9 +3501,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> else if (fs_info->compress_type == BTRFS_COMPRESS_ZSTD)
> features |= BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD;
>
> - if (features & BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA)
> - btrfs_info(fs_info, "has skinny extents");
> -
> /*
> * Flag our filesystem as having big metadata blocks if they are bigger
> * than the page size.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] btrfs: remove skinny extent verbose message
2022-06-23 8:22 ` Qu Wenruo
2022-06-23 8:30 ` Nikolay Borisov
@ 2022-06-23 14:19 ` David Sterba
2022-06-23 22:46 ` Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message") Qu Wenruo
1 sibling, 1 reply; 11+ messages in thread
From: David Sterba @ 2022-06-23 14:19 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Nikolay Borisov, linux-btrfs
On Thu, Jun 23, 2022 at 04:22:27PM +0800, Qu Wenruo wrote:
>
>
> On 2022/6/23 16:08, Nikolay Borisov wrote:
> > Skinny extents have been a default mkfs feature since version 3.18 i
> > (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
> > make skinny-metadata default") ). It really doesn't bring any value to
> > users to simply remove it.
> >
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>
> Looks fine to me.
>
> But this means we need to define the level of (in)compat flags we want
> to show in the dmesg.
Yes and we haven't done that so far so we should have some guidelines.
>
> By default, we have the following lines:
>
> BTRFS info (device loop0): flagging fs with big metadata feature
> BTRFS info (device loop0): using free space tree
> BTRFS info (device loop0): has skinny extents
> BTRFS info (device loop0): enabling ssd optimizations
> BTRFS info (device loop0): checking UUID tree
>
> For "big metadata" it's even less meaningful, and it doesn't even have
> sysfs entry for it.
There's an entry in the global features but 'big_metadata' does not
appear in the per-filesystem directory.
> For "free space tree" it may be helpful, but if one is really concerning
> about the cache version we're using, it's better to go sysfs other than
> checking the kernel dmesg.
The logged messages are a bit different as they could be stored and then
used for analysis. For new features it makes more sense to log them, I
think eg. the free space tree messages have been useful when debugging
the online conversion that took a few patches to get right.
> For "SSD", it's a good thing to output.
Agreed.
> For "UUID" tree, it's definitely not useful, even for developers.
Agreed.
> For skinny metadata it's the one you're cleaning.
Though I've sent patch to make it only debug I agree that this has
little value and don't object to removing it completely.
> So I guess you can clean up more unnecessary mount messages then?
The criteria I'd use for adding/removing the messages are vaguely like
that:
- does the user want to know a particular feature is in use? this is
namely when we're introducing something and want to verify what's
happening
- can the option have an impact on the filesystem behaviour? eg. like
ssd, but we tend to log almost all mount options already
- if there's a value for developer, the level should be debug, otherwise
info
- remove messages if a feature is on by default for a long time and does
not bring any other value, eg. the mixed_backref, big_metadata or
skinny extents;
the respective sysfs files may need to stay as they could be used for
runtime detection even after a long time, eg. in some testsuite
collecting testcases over time but likely not updating them, removal
should be done on case-by-case basis
^ permalink raw reply [flat|nested] 11+ messages in thread
* Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message")
2022-06-23 14:19 ` David Sterba
@ 2022-06-23 22:46 ` Qu Wenruo
2022-06-24 9:10 ` Forza
2022-06-24 10:52 ` Martin Steigerwald
0 siblings, 2 replies; 11+ messages in thread
From: Qu Wenruo @ 2022-06-23 22:46 UTC (permalink / raw)
To: dsterba, Nikolay Borisov, linux-btrfs
On 2022/6/23 22:19, David Sterba wrote:
> On Thu, Jun 23, 2022 at 04:22:27PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2022/6/23 16:08, Nikolay Borisov wrote:
>>> Skinny extents have been a default mkfs feature since version 3.18 i
>>> (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
>>> make skinny-metadata default") ). It really doesn't bring any value to
>>> users to simply remove it.
>>>
>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>
>> Looks fine to me.
>>
>> But this means we need to define the level of (in)compat flags we want
>> to show in the dmesg.
>
> Yes and we haven't done that so far so we should have some guidelines.
>>
>> By default, we have the following lines:
>>
>> BTRFS info (device loop0): flagging fs with big metadata feature
>> BTRFS info (device loop0): using free space tree
>> BTRFS info (device loop0): has skinny extents
>> BTRFS info (device loop0): enabling ssd optimizations
>> BTRFS info (device loop0): checking UUID tree
>>
>> For "big metadata" it's even less meaningful, and it doesn't even have
>> sysfs entry for it.
>
> There's an entry in the global features but 'big_metadata' does not
> appear in the per-filesystem directory.
>
>> For "free space tree" it may be helpful, but if one is really concerning
>> about the cache version we're using, it's better to go sysfs other than
>> checking the kernel dmesg.
>
> The logged messages are a bit different as they could be stored and then
> used for analysis. For new features it makes more sense to log them, I
> think eg. the free space tree messages have been useful when debugging
> the online conversion that took a few patches to get right.
>
>> For "SSD", it's a good thing to output.
>
> Agreed.
>
>> For "UUID" tree, it's definitely not useful, even for developers.
>
> Agreed.
>
>> For skinny metadata it's the one you're cleaning.
>
> Though I've sent patch to make it only debug I agree that this has
> little value and don't object to removing it completely.
>
>> So I guess you can clean up more unnecessary mount messages then?
>
> The criteria I'd use for adding/removing the messages are vaguely like
> that:
>
> - does the user want to know a particular feature is in use? this is
> namely when we're introducing something and want to verify what's
> happening
I'd say this is not that important.
In fact, there is a pretty bad example from the past, BIG_METADATA.
It's just we're supporting larger nodesize, it doesn't even bother users
that much, nor really need a incompat flag at all.
Another example is from recent subpage feature, it doesn't need any
incompat/compat RO flags at all, the only reason we're outputting
message for subpage is, it's not tested as much as native page size.
If we can ensure enough test coverage (already getting better and better
coverage) that experimental message would definitely go away.
Thus my idea criteria would be:
- Would this feature bring bad impact to end users?
If the feature is only bringing good impact, it should not be output.
Thus in this way, v2 cache nowadays should also be skipped, as it's
already well tested, and no real disadvantage at all.
>
> - can the option have an impact on the filesystem behaviour? eg. like
> ssd, but we tend to log almost all mount options already
>
> - if there's a value for developer, the level should be debug, otherwise
> info
I'd say, considering how hard to enable debug messages, it doesn't
really make much sense for developers.
Thus unfortunately such debugging info would still better to be at info
level, just in case it's something like dying message and/or the user
can not easily reproduce it.
But we may want a much shorter (even it means less human readable), less
noisy output.
Thus I'd say, we may want to output hex incompat/compat RO/compat flags
just in one line during mount, instead of current one feature per-line
behavior.
By this, it provides more debug info, but still way shorter, and way
more expandable.
Thanks,
Qu
>
> - remove messages if a feature is on by default for a long time and does
> not bring any other value, eg. the mixed_backref, big_metadata or
> skinny extents;
> the respective sysfs files may need to stay as they could be used for
> runtime detection even after a long time, eg. in some testsuite
> collecting testcases over time but likely not updating them, removal
> should be done on case-by-case basis
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message")
2022-06-23 22:46 ` Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message") Qu Wenruo
@ 2022-06-24 9:10 ` Forza
2022-06-24 10:52 ` Martin Steigerwald
1 sibling, 0 replies; 11+ messages in thread
From: Forza @ 2022-06-24 9:10 UTC (permalink / raw)
To: Qu Wenruo, dsterba, Nikolay Borisov, linux-btrfs
On 6/24/22 00:46, Qu Wenruo wrote:
>
>
> On 2022/6/23 22:19, David Sterba wrote:
>> On Thu, Jun 23, 2022 at 04:22:27PM +0800, Qu Wenruo wrote:
>>>
>>>
>>> On 2022/6/23 16:08, Nikolay Borisov wrote:
>>>> Skinny extents have been a default mkfs feature since version 3.18 i
>>>> (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
>>>> make skinny-metadata default") ). It really doesn't bring any value to
>>>> users to simply remove it.
>>>>
>>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>>
>>> Looks fine to me.
>>>
>>> But this means we need to define the level of (in)compat flags we want
>>> to show in the dmesg.
>>
>> Yes and we haven't done that so far so we should have some guidelines.
>>>
>>> By default, we have the following lines:
>>>
>>> BTRFS info (device loop0): flagging fs with big metadata feature
>>> BTRFS info (device loop0): using free space tree
>>> BTRFS info (device loop0): has skinny extents
>>> BTRFS info (device loop0): enabling ssd optimizations
>>> BTRFS info (device loop0): checking UUID tree
>>>
>>> For "big metadata" it's even less meaningful, and it doesn't even have
>>> sysfs entry for it.
>>
>> There's an entry in the global features but 'big_metadata' does not
>> appear in the per-filesystem directory.
>>
>>> For "free space tree" it may be helpful, but if one is really concerning
>>> about the cache version we're using, it's better to go sysfs other than
>>> checking the kernel dmesg.
>>
>> The logged messages are a bit different as they could be stored and then
>> used for analysis. For new features it makes more sense to log them, I
>> think eg. the free space tree messages have been useful when debugging
>> the online conversion that took a few patches to get right.
>>
>>> For "SSD", it's a good thing to output.
>>
>> Agreed.
>>
>>> For "UUID" tree, it's definitely not useful, even for developers.
>>
>> Agreed.
>>
>>> For skinny metadata it's the one you're cleaning.
>>
>> Though I've sent patch to make it only debug I agree that this has
>> little value and don't object to removing it completely.
>>
>>> So I guess you can clean up more unnecessary mount messages then?
>>
>> The criteria I'd use for adding/removing the messages are vaguely like
>> that:
>>
>> - does the user want to know a particular feature is in use? this is
>> namely when we're introducing something and want to verify what's
>> happening
I think this is a good thought.
>
> I'd say this is not that important.
>
> In fact, there is a pretty bad example from the past, BIG_METADATA.
>
> It's just we're supporting larger nodesize, it doesn't even bother users
> that much, nor really need a incompat flag at all.
>
> Another example is from recent subpage feature, it doesn't need any
> incompat/compat RO flags at all, the only reason we're outputting
> message for subpage is, it's not tested as much as native page size.
>
> If we can ensure enough test coverage (already getting better and better
> coverage) that experimental message would definitely go away.
>
>
> Thus my idea criteria would be:
>
> - Would this feature bring bad impact to end users?
> If the feature is only bringing good impact, it should not be output.
>
> Thus in this way, v2 cache nowadays should also be skipped, as it's
> already well tested, and no real disadvantage at all.
>
Even if v2 is default, lots of users are on older kernel/progs and would
still benefit from seeing these messages.
>>
>> - can the option have an impact on the filesystem behaviour? eg. like
>> ssd, but we tend to log almost all mount options already
>>
>> - if there's a value for developer, the level should be debug, otherwise
>> info
>
> I'd say, considering how hard to enable debug messages, it doesn't
> really make much sense for developers.
>
> Thus unfortunately such debugging info would still better to be at info
> level, just in case it's something like dying message and/or the user
> can not easily reproduce it.
>
> But we may want a much shorter (even it means less human readable), less
> noisy output.
>
> Thus I'd say, we may want to output hex incompat/compat RO/compat flags
> just in one line during mount, instead of current one feature per-line
> behavior.
>
> By this, it provides more debug info, but still way shorter, and way
> more expandable.
>
The idea of having a short feature flag code is good. As an end-user I
do want to have the human readable form too. Especially during
transition periods where lots of distros still use older kernels that
may not have the new features enabled. For example a lot of users still
use Ubuntu with a 5.4 kernel.
Of course, we should teach users to use /sys/ information or use btrfs
inspect-internal tools, but I think this will take a long time.
> Thanks,
> Qu
>>
>> - remove messages if a feature is on by default for a long time and does
>> not bring any other value, eg. the mixed_backref, big_metadata or
>> skinny extents;
>> the respective sysfs files may need to stay as they could be used for
>> runtime detection even after a long time, eg. in some testsuite
>> collecting testcases over time but likely not updating them, removal
>> should be done on case-by-case basis
The sysfs files could stay always? Is that a problem?
Thanks,
Forza
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message")
2022-06-23 22:46 ` Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message") Qu Wenruo
2022-06-24 9:10 ` Forza
@ 2022-06-24 10:52 ` Martin Steigerwald
2022-06-24 13:10 ` Forza
1 sibling, 1 reply; 11+ messages in thread
From: Martin Steigerwald @ 2022-06-24 10:52 UTC (permalink / raw)
To: dsterba, Nikolay Borisov, linux-btrfs, Qu Wenruo
Qu Wenruo - 24.06.22, 00:46:02 CEST:
> Thus my idea criteria would be:
>
> - Would this feature bring bad impact to end users?
> If the feature is only bringing good impact, it should not be
> output.
>
> Thus in this way, v2 cache nowadays should also be skipped, as it's
> already well tested, and no real disadvantage at all.
There is one aspect where I find those messages helpful:
To confirm that I successfully enabled a feature (like space cache v2).
However, for that there does not really need to be a kernel message on
each mounting, as long as I have a way to confirm the currently used
features of BTRFS like:
- which checksum algorithm?
- space cache v2
- big metadata
- which compression method configured as standard (I am aware that does
not say anything about which files are compressed by which method)
and all other things like that.
Preferably in the output of just one command instead of being scattered
around several different outputs or not even available at all. For
example I am not aware of a command that confirms whether or not a
filesystem uses "xxhash" instead of "crc32c" as checksum algorithm.
Something a bit similar to "tune2fs -l" or "xfs_info".
Is there such a something? I believe there is some way to dump a
superblock however is there something that gives a structured output on
what features are in use on a given filesystem?
Best,
--
Martin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message")
2022-06-24 10:52 ` Martin Steigerwald
@ 2022-06-24 13:10 ` Forza
0 siblings, 0 replies; 11+ messages in thread
From: Forza @ 2022-06-24 13:10 UTC (permalink / raw)
To: Martin Steigerwald, dsterba, Nikolay Borisov, linux-btrfs, Qu Wenruo
On 6/24/22 12:52, Martin Steigerwald wrote:
> Qu Wenruo - 24.06.22, 00:46:02 CEST:
>> Thus my idea criteria would be:
>>
>> - Would this feature bring bad impact to end users?
>> If the feature is only bringing good impact, it should not be
>> output.
>>
>> Thus in this way, v2 cache nowadays should also be skipped, as it's
>> already well tested, and no real disadvantage at all.
>
> There is one aspect where I find those messages helpful:
>
> To confirm that I successfully enabled a feature (like space cache v2).
>
> However, for that there does not really need to be a kernel message on
> each mounting, as long as I have a way to confirm the currently used
> features of BTRFS like:
>
> - which checksum algorithm?
> - space cache v2
> - big metadata
> - which compression method configured as standard (I am aware that does
> not say anything about which files are compressed by which method)
>
> and all other things like that.
>
> Preferably in the output of just one command instead of being scattered
> around several different outputs or not even available at all. For
> example I am not aware of a command that confirms whether or not a
> filesystem uses "xxhash" instead of "crc32c" as checksum algorithm.
>
> Something a bit similar to "tune2fs -l" or "xfs_info".
>
> Is there such a something? I believe there is some way to dump a
> superblock however is there something that gives a structured output on
> what features are in use on a given filesystem?
>
This is perhaps the better choice? A "btrfs info" or "btrfs filesystem
info" command could output all these things in a user friendly and
machine readable way. Possibly with added machine parsable output with
an optional flag.
Currently we have "btrfs inspect-internal dump-super" which has a lot of
the infomation already. I might be mistaken, but I think it only reads
the superblocks directly off the block device, and it may not reflect
what options the kernel currently uses. For example it cannot tell what
kernel implementation of the checksum algorithm is used (crc32c-intel,
sha256-generic, etc).
# btrfs inspect-internal dump-super /dev/sdc2
superblock: bytenr=65536, device=/dev/sdc2
---------------------------------------------------------
csum_type 1 (xxhash64)
csum_size 8
csum 0x46c8eb9318dc87eb [match]
bytenr 65536
flags 0x1
( WRITTEN )
magic _BHRfS_M [match]
fsid aa358efb-ce43-498c-9997-0d35ba13261f
metadata_uuid aa358efb-ce43-498c-9997-0d35ba13261f
label btrfs_backup_2T
generation 30096
root 927851757568
sys_array_size 129
chunk_root_generation 30092
root_level 1
chunk_root 23003136
chunk_root_level 1
log_root 0
log_root_transid 0
log_root_level 0
total_bytes 2996496760832
bytes_used 1756499472384
sectorsize 4096
nodesize 16384
leafsize (deprecated) 16384
stripesize 4096
root_dir 6
num_devices 1
compat_flags 0x0
compat_ro_flags 0x3
( FREE_SPACE_TREE |
FREE_SPACE_TREE_VALID )
incompat_flags 0x371
( MIXED_BACKREF |
COMPRESS_ZSTD |
BIG_METADATA |
EXTENDED_IREF |
SKINNY_METADATA |
NO_HOLES )
cache_generation 0
uuid_tree_generation 30096
block_group_root 0
block_group_root_generation 0
block_group_root_level 0
dev_item.uuid d97d96ae-8f9c-4d32-a326-632700d29fa2
dev_item.fsid aa358efb-ce43-498c-9997-0d35ba13261f [match]
dev_item.type 0
dev_item.total_bytes 2996496760832
dev_item.bytes_used 1803903041536
dev_item.io_align 4096
dev_item.io_width 4096
dev_item.sector_size 4096
dev_item.devid 1
dev_item.dev_group 0
dev_item.seek_speed 0
dev_item.bandwidth 0
dev_item.generation 0
Thanks,
Forza
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] btrfs: remove skinny extent verbose message
2022-06-23 8:08 [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
` (2 preceding siblings ...)
2022-06-23 9:52 ` Qu Wenruo
@ 2022-06-24 16:21 ` David Sterba
3 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2022-06-24 16:21 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Thu, Jun 23, 2022 at 11:08:58AM +0300, Nikolay Borisov wrote:
> Skinny extents have been a default mkfs feature since version 3.18 i
> (introduced in btrfs-progs commit 6715de04d9a7 ("btrfs-progs: mkfs:
> make skinny-metadata default") ). It really doesn't bring any value to
> users to simply remove it.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-06-24 16:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-23 8:08 [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
2022-06-23 8:22 ` Qu Wenruo
2022-06-23 8:30 ` Nikolay Borisov
2022-06-23 14:19 ` David Sterba
2022-06-23 22:46 ` Criteria for mount messages. (was "Re: [PATCH] btrfs: remove skinny extent verbose message") Qu Wenruo
2022-06-24 9:10 ` Forza
2022-06-24 10:52 ` Martin Steigerwald
2022-06-24 13:10 ` Forza
2022-06-23 9:30 ` [PATCH] btrfs: remove skinny extent verbose message Nikolay Borisov
2022-06-23 9:52 ` Qu Wenruo
2022-06-24 16:21 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.