Re: [PATCH 1/2] btrfs: remove MIXED_BACKREF sysfs file

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/6/24 23:44, David Sterba wrote:
> On Fri, Jun 24, 2022 at 10:02:43PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2022/6/24 21:47, David Sterba wrote:
>>> On Fri, Jun 24, 2022 at 07:46:12PM +0800, Qu Wenruo wrote:
>>>> On 2022/6/24 19:32, Nikolay Borisov wrote:
>>>>> On 24.06.22 г. 11:13 ч., Qu Wenruo wrote:
>>>>>>
>>>>>> I don't think that's the correct way to go.
>>>>>>
>>>>>> In fact, I think sysfs should have everything, no matter how long
>>>>>> supported it is.
>>>>>
>>>>> I disagree, for things which are considered stand alone features - yes.
>>>>> Like free space tree 2, but for something like backrefs, heck I think
>>>>> we've even removed code which predates mixed backrefs so I'm not
>>>>> entirely use the filesystem can function with that feature turned off,
>>>>> actually it's not possible to create a non-mixedbackref file system
>>>>> since this behavior is hard-coded in btrfs-progs. Also the commit for
>>>>> the backrefs states:
>>>>>
>>>>>
>>>>> This commit introduces a new kind of back reference for btrfs metadata.
>>>>> Once a filesystem has been mounted with this commit, IT WILL NO LONGER
>>>>> BE MOUNTABLE BY OLDER KERNELS.
>>>>
>>>> That means we're hiding incompat features from the user.
>>>>
>>>> Even if it's not tunable and should always be enabled, we still need to
>>>> add that.
>>>
>>> I think the mixed_backref is an exception because it's been part of the
>>> default format for so long that we don't even remember there was
>>> something else. For users it does not mean anything today, moreover it
>>> could be confused with mixed block groups.
>>
>> Then after some time, there will be some "smart" users find that we have
>> one incompat bit without any explanation.
>
> Removed functionality is documented, the sysfs feature files are in
> manual pages and we can add a notice in which version it was removed.

Then have all the old features get removed one by one, until one day we
have a dozen of bits set, but only one or two still show in sysfs features?

No, this definitely doesn't look sane to me.

It's just trying to hide some bad behaviors which we didn't get it right
in the first place.
It's fine to didn't get those things done correct in the first place,
but not fine to hide them.

Especially those sysfs is already hidden to most users, way less
invasive than the dmesg output/mkfs features/etc, why we still want to
remove them?

Thanks,
Qu