Re: [PATCH v2 3/4] btrfs: add per-inode compression levels in xattrs

On Tue Aug 11, 2026 at 5:21 AM CEST, Qu Wenruo wrote:
>
>
> 在 2026/8/11 12:25, koraynilay 写道:
>>> I'd prefer to have a dedicated patch to set compress_level to the
>>> default value 0, as a proper bug fix as the first patch of the series.
>>>
>>> As you mentioned in the cover-letter, this is in fact fixing a bug in
>>> the old behavior (mismatched algo and level).
>>>
>>> So it's definitely worth a dedicated fix, so that we can backport the
>>> fix without pulling in the full series for older kernels.
>> 
>> Ok so, after thinking about it more and bouncing ideas around (with Zygo
>> too) I realized one thing, that while it *is* technically a bug, I don't
>> think it's a bug worth backporting.
>> 
>> My reasoning is simply that there is no use-case where the current
>> "buggy" behaviour would be damaging, as the levels get clamped to the
>> supported range anyway, while arguably there are (albeit very rare and
>> probably not very smart in the first place) use-cases where fixing it
>> could be somewhat (limitedly) damaging.
>
> To be honest, if the current behavior is not damaging, which I agree, 
> then it's also not damaging to use the default level.
>
> After all, it's just a level change, which is never damaging.

Yeah, I meant that while both aren't damaging, option 3 would be a
*little* more damaging vs option 1 (as per https://xkcd.com/1172), since
with option 1 nothing can break at all (because no change would be made
on older kernels).
(For this I mean only the bugfix of course).

>> 
>> More importantly IMO, doing this would allow us to explicitly explain
>> the currently undocumented behaviour in the btrfs-property(8) manpage as
>> "just so you know, for kernel versions < 7.x cross-algo level leakage
>> from -o compress was happening".
>
> Which also applies to option 3.

Option 3 would mean the behaviour can be different depending on the
kernel < 7.X having the backported patch or not tho, so we can't say
that with 100% certainty in the manpage, but it would need to also say
"this is true only if you don't have this specific bugfix patch, if your
kernel has it, then the behaviour is this other one".

>> 
>> 
>> 
>> As for how to handle it after having support for levels in the XATTR,
>> option 2, aka leaking the compress level only if the algo matches, would
>> be the best imo:
>> 
>> Example use-case:
>> 
>> - /fs has various types of files, from media to git repos, that would
>>    benefit from the normal compress mount option;
>> - /fs also has big virtual machine disks, that have very compressible
>>    parts but also very uncompressible parts;
>> 
>> using only `mount -o compress=zstd:7 /fs` may mark the vm disks with
>> NOCOMPRESS as soon as an incompressible extent gets found, but setting
>> btrfs.compression=zstd won't, as it will try to compress every extent
>> anyway[1].
>
> BTW, the default level is 3, so 7 is already trying to compress harder 
> than default.
> (At least from the official man page)
>

Yes, I could've used 15 too, it was just an example of a non-default
level specified in the compress option that would then get kept for
specific inodes that have the property set without any level specified.

>> This way if the user intends to change the compress level for the whole
>> fs, they can just change the mount option, knowing that the new level
>> will apply to the (new) vm extents too, like it will for all other files.
>> 
>> In this example "btrfs.compression=zstd" and "btrfs.compression=zstd:0"
>> would behave the same, which means the file's extent will get compressed
>> with zstd:7, but when the user remounts with e.g. zstd:15, they will use
>> this new level (only for the extents written from that point afterwards,
>> of course).
>
> I'd say, in this particular case, user should specify a different level 
> for VM images, after the level support in XATTR, other than relying on 
> the global mount option level.
>

But if the user wants to have that data be compressed in the same way as
the rest of the fs, they'd have to change the XATTR every time they want
to change the level.

>> 
>> (I'm ignoring the case where the user wants to change the algorithm and
>> let the vms inherit it, as for this specific use-case that would likely
>> require a whole new feature/property to say "try to compress anyway but
>> not as much as compress-force" and probably most people use zstd anyway
>> nowadays).
>> 
>> 
>> 
>> After coming to this conclusion, I'm personally pretty satisfied with
>> this solution, while I wasn't as much with the other ones.
>
> Since my idea is pretty different on option 2 vs 3, and I do not find we 
> can persuade each other, so I'll leave David to do the final call.
>

That's fair, at this point I thought about this so much all options have
pros and cons and I switched from preferring 3, to preferring 2, to now
preferring 1 a bit more (especially because of the docs clarity).

Thanks

Best,
koraynilay

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQSgVimKafU5DQMcjcDmj22qf5IGXAUCansZ9AAKCRDmj22qf5IG
XHf4AP4zOu54oNpUTbJvUBqHeeqCsHqMFfxfhP0OvhBvHPf1ggEA9dw30d+op1BW
b8l/VzjCrCdr8T0QaXHMCYZhKrdCOAY=
=CIZ8
-----END PGP SIGNATURE-----