Re: [PATCH] btrfs/defrag: implement compression levels

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

在 2025/3/5 18:31, David Sterba 写道:
> On Wed, Mar 05, 2025 at 06:14:16PM +1030, Qu Wenruo wrote:
>> [...]
>>>>>         /* spare for later */
>>>>>         __u32 unused[4];
>>>>
>>>> We have enough space left here, although u32 is overkilled for
>>>> compress_type, using the unused space for a new s8/s16/s32 member should
>>>> be fine.
>>>
>>> That is what I did originally, but discussing with Dave he suggested
>>> this solution.
>>
>> Normally I would be fine with the union, to save some memory.
>>
>> Maybe I'm a little paranoid, but the defrag ioctl flag check is only
>> introduced last year by commit 173431b274a9 ("btrfs: defrag: reject
>> unknown flags of btrfs_ioctl_defrag_range_args").
>
> The commit has been backported to stable trees 4.19.307 5.10.210 5.15.149
> 5.4.269 6.1.76 6.6.15 6.7.3 , so we could assume the flags are
> validated.

I know it's backported to all stable kernels, but I still won't consider
a patch that's only one year old to be applied to all the kernels in the
wide.

Maybe I'm really paranoid on this.

>
>> So it's possible that some older kernels don't have that commit, and may
>> incorrectly continue by ignoring the flag.
>> Thankfully that should fail with -EINVAL (type always in the higher
>> bits, thus always tricking the NR_COMPRESS_TYPES check.
>>
>> If that layout (type in higher bits, level in lower bits) is
>> intentionally, I'd say it's very clever.
>>
>> Anyway either solution looks fine to me now.
>
> The layout also depends on endianness, but should not matter as long as
> the flgags are validated. If not, either the level is ignored or it
> fails due to the >= NR_COMPRESS_TYPES check. Both should be acceptable
> as fallback.

Since the fallback behavior is sane, I'm fine with the union solution.

Thanks,
Qu