On Tue Nov 10, 2026 at 3:50 AM CET, Sterling Windmill wrote: >> I read the code a bit (and bounced some ideas with gemini) and I think a >> "force:algo:level" would work well. >> >> This, compared to "algo:level:force", would also allow a "force" value >> (handled like "none" is already handled), that lets the user set *only* >> the compress-force, without having to specify algo:level, so inheriting >> from -o compress=, and the parsing would be as simple as if ("force") >> str += 6, where str is the xattr string that gets then parsed by the >> rest of the code (in compression_apply()). > > Thanks, a bare "force" value is a nice thing to have, and I agree the > separate property isn't worth it. > > From reading the code only, a few things stand out for the bare > "force" case, where prop_compress would stay 0: > > - compress_file_range() starts from fs_info->compress_type, so on a > filesystem mounted without compress= there is no algorithm to use. > It needs a defined behaviour, such as falling back to zlib as the > chattr +c path does, or having no effect. > > - The check at mark_incompressible only looks at FORCE_COMPRESS and > prop_compress, so a failed attempt would set NOCOMPRESS and leave > it behind after force is removed. > > - btrfs_compress_typelevel2str() returns NULL when prop_compress is > 0, so children wouldn't inherit it and btrfs_fileattr_set() would > fall back to the mount algorithm. That helper would also need to > emit "force" in whichever form we choose. Ah yes, these would all need to be changed (but only small changes AFAICT) for a "force" patch to work. > One thing I should have mentioned: Qu pointed out on v1 that older > kernels ignore an appended ":" because apply only compares the > leading bytes. "zstd:3:force" would keep that property, with older > kernels treating it as plain zstd, while "force:zstd:3" and a bare > "force" would get -EINVAL from both validate and apply. Is bare > "force" worth that trade-off, or would the suffix be better? I think it's a worth trade-off, and my patch series already will break some setups where users had "zstd:random_text" (which would be semantically invalid anyway). > If we go with the prefix, the match probably needs to be exactly > "force:", in both validate and apply, and "force:none" should be > rejected. Yes, my current thinking is handling "force" exactly like "none"/"no" is handled right now, with an additional else if for "force:" which moves the string pointer to +6, which gets then passed to the rest of the checks as they are in my patch series. >> It should probably be a separate patch, but it'd depend on my patch >> series, so not sure if I should just make a v7 with that or not. > > I'd lean towards a separate patch on top rather than a v7, since this > is a new feature that may need its own discussion, and it shouldn't > hold up your series. I think adding a force patch to my series wouldn't be a problem, as my series has been finalized since september, so I doubt it will change much in merging time. > David, do you have a preference? And if the series isn't likely to > land soon, would you rather see force as a small standalone patch? > > I'm happy to write the force patch on top, or to test yours on the > images from my table, whichever you prefer. Yes, please test mine and also write a patch on top of my series, I'll be happy to test that on my end as well. After that we can decide if it's better to include it in a v7 or send it as a standalone patch with references to my v6. Thanks, koraynilay > Thanks, > Sterling