Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [RFC] btrfs: forcing compression per inode via the compression property?
@ 2026-11-09 13:51 Sterling Windmill
  2026-11-09 14:00 ` koraynilay
  0 siblings, 1 reply; 7+ messages in thread
From: Sterling Windmill @ 2026-11-09 13:51 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba, koraynilay

(Resending as plain text; the list rejected my first copy as HTML.
Sorry for the duplicate to those on Cc.)

Hi,

compress-force can only be set for the whole filesystem. I would like
to apply it to one subvolume and leave the rest of the filesystem on
plain compress. Before writing a patch, I'd like to know whether this
would be welcome and what form it should take.

My filesystem (kernel 7.3.3, 10-device RAID10) holds both raw disk
image backups and large, already-compressed video files, and these
want opposite settings.

The images need compress-force. I copied one image (about 60G of data
in a sparse 249G file) three times with
"cp --reflink=never --sparse=always". Sizes are from compsize:

  mode                      extents   on disk   stored raw  zstd in->out
  compress-force=zstd:3     615,577     23G        11G      48G -> 12G
  compress=zstd:3 + prop    492,796     31G        27G      33G -> 4.6G
  compress=zstd:3           218,911     60G        60G     944K -> 296K

"prop" is btrfs.compression=zstd. The last copy ended up with
NOCOMPRESS set. This is one image and one sequential write, so I would
not read much into the exact percentages, but the property costs about
35% more space here than compress-force. I have not measured how much
of that comes from the heuristic's sampling and how much from data
that is compressible despite its byte distribution.

The video pays for compress-force. An 82G file had 169,776 extents and
read sequentially at 270 MB/s. After "btrfs filesystem defragment
--nocomp" it had 666 extents and read at 622 MB/s. That works, but it
has to be repeated for new files and it breaks reflinks.

As I read inode_need_compress(), FORCE_COMPRESS is only ever taken from
the mount option, and a property always goes through
btrfs_compress_heuristic(). compress_file_range() already avoids
setting NOCOMPRESS when prop_compress is set, so a per-inode force
seems to need little more than this (hypothetical field, untested):

	if (btrfs_test_opt(fs_info, FORCE_COMPRESS) ||
	    inode->prop_compress_force)
		return 1;

With that, I could mount with compress=zstd:3 and set force on the
backup subvolume, relying on the usual property inheritance.

Questions:

1. Is a per-inode force something you would accept, or would you
   rather see the heuristic improved for this kind of data?

2. What should the interface be? The per-inode compression levels
   series [1] adds "algo:level" to the same property. Would a suffix
   such as "zstd:3:force" be acceptable, or a separate property? How
   much does the behaviour of older kernels reading the value
   constrain this?

3. How should it interact with NOCOMPRESS? I assume setting force
   would clear the flag and take precedence over it. The 2020 thread
   on compress-force and per-inode settings seems to have left the
   three-state question open:
   https://lore.kernel.org/all/b92d0141-f68f-ce96-8099-e145ebc6f594@toxicpanda.com/T/

4. Should this wait until the levels series is settled?

If there is interest, I am happy to write the patch and test it on
this data.

Thanks,
Sterling Windmill

[1] https://lwn.net/Articles/1096934/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] btrfs: forcing compression per inode via the compression property?
  2026-11-09 13:51 [RFC] btrfs: forcing compression per inode via the compression property? Sterling Windmill
@ 2026-11-09 14:00 ` koraynilay
  2026-11-09 14:14   ` Sterling Windmill
  0 siblings, 1 reply; 7+ messages in thread
From: koraynilay @ 2026-11-09 14:00 UTC (permalink / raw)
  To: Sterling Windmill, linux-btrfs; +Cc: David Sterba, koraynilay

[-- Attachment #1: Type: text/plain, Size: 3742 bytes --]

On Mon Nov 9, 2026 at 2:51 PM CET, Sterling Windmill wrote:
> (Resending as plain text; the list rejected my first copy as HTML.
> Sorry for the duplicate to those on Cc.)
>
> Hi,
>
> compress-force can only be set for the whole filesystem. I would like
> to apply it to one subvolume and leave the rest of the filesystem on
> plain compress. Before writing a patch, I'd like to know whether this
> would be welcome and what form it should take.
>
> My filesystem (kernel 7.3.3, 10-device RAID10) holds both raw disk
> image backups and large, already-compressed video files, and these
> want opposite settings.
>
> The images need compress-force. I copied one image (about 60G of data
> in a sparse 249G file) three times with
> "cp --reflink=never --sparse=always". Sizes are from compsize:
>
>   mode                      extents   on disk   stored raw  zstd in->out
>   compress-force=zstd:3     615,577     23G        11G      48G -> 12G
>   compress=zstd:3 + prop    492,796     31G        27G      33G -> 4.6G
>   compress=zstd:3           218,911     60G        60G     944K -> 296K
>
> "prop" is btrfs.compression=zstd. The last copy ended up with
> NOCOMPRESS set. This is one image and one sequential write, so I would
> not read much into the exact percentages, but the property costs about
> 35% more space here than compress-force. I have not measured how much
> of that comes from the heuristic's sampling and how much from data
> that is compressible despite its byte distribution.
>
> The video pays for compress-force. An 82G file had 169,776 extents and
> read sequentially at 270 MB/s. After "btrfs filesystem defragment
> --nocomp" it had 666 extents and read at 622 MB/s. That works, but it
> has to be repeated for new files and it breaks reflinks.
>
> As I read inode_need_compress(), FORCE_COMPRESS is only ever taken from
> the mount option, and a property always goes through
> btrfs_compress_heuristic(). compress_file_range() already avoids
> setting NOCOMPRESS when prop_compress is set, so a per-inode force
> seems to need little more than this (hypothetical field, untested):
>
> 	if (btrfs_test_opt(fs_info, FORCE_COMPRESS) ||
> 	    inode->prop_compress_force)
> 		return 1;
>
> With that, I could mount with compress=zstd:3 and set force on the
> backup subvolume, relying on the usual property inheritance.
>
> Questions:
>
> 1. Is a per-inode force something you would accept, or would you
>    rather see the heuristic improved for this kind of data?
>
> 2. What should the interface be? The per-inode compression levels
>    series [1] adds "algo:level" to the same property. Would a suffix
>    such as "zstd:3:force" be acceptable, or a separate property? How
>    much does the behaviour of older kernels reading the value
>    constrain this?
>
> 3. How should it interact with NOCOMPRESS? I assume setting force
>    would clear the flag and take precedence over it. The 2020 thread
>    on compress-force and per-inode settings seems to have left the
>    three-state question open:
>    https://lore.kernel.org/all/b92d0141-f68f-ce96-8099-e145ebc6f594@toxicpanda.com/T/
>
> 4. Should this wait until the levels series is settled?
>
> If there is interest, I am happy to write the patch and test it on
> this data.

Hello, there is a github issue tracking this and a patch[2] (by me)
currently pending final review.

Can you see if [2] has the same behaviour you expect?

[1]: https://github.com/kdave/btrfs-progs/issues/329
[2]: https://lore.kernel.org/linux-btrfs/20261103194734.598182-1-koray.fra@gmail.com/T/#t

Best,
koraynilay

> Thanks,
> Sterling Windmill
>
> [1] https://lwn.net/Articles/1096934/



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] btrfs: forcing compression per inode via the compression property?
  2026-11-09 14:00 ` koraynilay
@ 2026-11-09 14:14   ` Sterling Windmill
  2026-11-09 20:25     ` koraynilay
  0 siblings, 1 reply; 7+ messages in thread
From: Sterling Windmill @ 2026-11-09 14:14 UTC (permalink / raw)
  To: koraynilay, linux-btrfs; +Cc: David Sterba

> Hello, there is a github issue tracking this and a patch[2] (by me)
> currently pending final review.
>
> Can you see if [2] has the same behaviour you expect?
>
> [1]: https://github.com/kdave/btrfs-progs/issues/329
> [2]: https://lore.kernel.org/linux-btrfs/20261103194734.598182-1-koray.fra@gmail.com/T/#t

Hi koraynilay,

Thanks for pointing me at it. From reading the series (I haven't built
or run it), it looks like it solves a related but different problem.

It lets the property carry a level, which is useful, but the only
inode.c changes are to how the level is chosen. inode_need_compress()
is untouched, so a file with btrfs.compression=zstd:3 would still go
through btrfs_compress_heuristic(), because FORCE_COMPRESS still only
comes from the mount option. I would expect to keep getting the 31G
result from my table, unless I've missed something.

What I'm after is a way for the property to skip the heuristic the
way compress-force does, so that one subvolume can behave like
compress-force on a filesystem mounted with plain compress.

Since your series defines the "algo:level" syntax, would something
like "zstd:3:force" fit as a follow-up on top of it? The alternative
I can see is a separate property, which would only need a small
change to inode_need_compress() and wouldn't depend on your series,
at the cost of having two properties for one setting. I'd be glad to
hear which you or others on the list would prefer.

Thanks,
Sterling

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] btrfs: forcing compression per inode via the compression property?
  2026-11-09 14:14   ` Sterling Windmill
@ 2026-11-09 20:25     ` koraynilay
  2026-11-10  2:50       ` Sterling Windmill
  0 siblings, 1 reply; 7+ messages in thread
From: koraynilay @ 2026-11-09 20:25 UTC (permalink / raw)
  To: Sterling Windmill, koraynilay, linux-btrfs; +Cc: David Sterba

[-- Attachment #1: Type: text/plain, Size: 2476 bytes --]

On Mon Nov 9, 2026 at 3:14 PM CET, Sterling Windmill wrote:
>> Hello, there is a github issue tracking this and a patch[2] (by me)
>> currently pending final review.
>>
>> Can you see if [2] has the same behaviour you expect?
>>
>> [1]: https://github.com/kdave/btrfs-progs/issues/329
>> [2]: https://lore.kernel.org/linux-btrfs/20261103194734.598182-1-koray.fra@gmail.com/T/#t
>
> Hi koraynilay,
>
> Thanks for pointing me at it. From reading the series (I haven't built
> or run it), it looks like it solves a related but different problem.
>
> It lets the property carry a level, which is useful, but the only
> inode.c changes are to how the level is chosen. inode_need_compress()
> is untouched, so a file with btrfs.compression=zstd:3 would still go
> through btrfs_compress_heuristic(), because FORCE_COMPRESS still only
> comes from the mount option. I would expect to keep getting the 31G
> result from my table, unless I've missed something.
>
> What I'm after is a way for the property to skip the heuristic the
> way compress-force does, so that one subvolume can behave like
> compress-force on a filesystem mounted with plain compress.
>
> Since your series defines the "algo:level" syntax, would something
> like "zstd:3:force" fit as a follow-up on top of it? The alternative

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()).

> I can see is a separate property, which would only need a small
> change to inode_need_compress() and wouldn't depend on your series,
> at the cost of having two properties for one setting. I'd be glad to
> hear which you or others on the list would prefer.

A separate property would probably be more work/less easy to handle, as
it also needs to get checked for conflicts like btrfs.compression=none
and btrfs.compression_force=yes.


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.

Best,
koraynilay

>
> Thanks,
> Sterling



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] btrfs: forcing compression per inode via the compression property?
  2026-11-09 20:25     ` koraynilay
@ 2026-11-10  2:50       ` Sterling Windmill
  2026-11-10  9:29         ` koraynilay
  0 siblings, 1 reply; 7+ messages in thread
From: Sterling Windmill @ 2026-11-10  2:50 UTC (permalink / raw)
  To: linux-btrfs, koraynilay; +Cc: David Sterba

> 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.

One thing I should have mentioned: Qu pointed out on v1 that older
kernels ignore an appended ":<level>" 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?

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.

> 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.

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.

Thanks,
Sterling

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] btrfs: forcing compression per inode via the compression property?
  2026-11-10  2:50       ` Sterling Windmill
@ 2026-11-10  9:29         ` koraynilay
  2026-11-12  6:15           ` Zygo Blaxell
  0 siblings, 1 reply; 7+ messages in thread
From: koraynilay @ 2026-11-10  9:29 UTC (permalink / raw)
  To: Sterling Windmill, linux-btrfs, koraynilay; +Cc: David Sterba

[-- Attachment #1: Type: text/plain, Size: 3667 bytes --]

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 ":<level>" 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



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] btrfs: forcing compression per inode via the compression property?
  2026-11-10  9:29         ` koraynilay
@ 2026-11-12  6:15           ` Zygo Blaxell
  0 siblings, 0 replies; 7+ messages in thread
From: Zygo Blaxell @ 2026-11-12  6:15 UTC (permalink / raw)
  To: koraynilay; +Cc: Sterling Windmill, linux-btrfs, David Sterba

On Tue, Nov 10, 2026 at 10:29:16AM +0100, koraynilay wrote:
> 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.

There is a symmetric gap to be filled here:  currently, there is no way
for individual files to opt out of a compress-force mount.  The force
mount option is checked first, so a compression attribute that says "none"
(or chattr +m) is ignored, and the file is compressed anyway (along with
all the side-effects that come with force, like 512K maximum uncompressed
extent size).

> > 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.

It might be nice to refactor all of those, so one parser gives you
a struct with compress type, level, and force, or gives a complete
string with all of those written out.  It would also get rid of
the "none" special case, which can't accept a suffix.

Doing it piecemeal means that someone's going to forget a parameter
somewhere, especially if it's extended over time, which is how we got
into the current mess of partial implementation everywhere.

> > One thing I should have mentioned: Qu pointed out on v1 that older
> > kernels ignore an appended ":<level>" 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).

Putting force first is more consistent with the mount option.
"compress-force=zstd:5" becomes "force:zstd:5" on the inode, almost
exactly the same string.  The mount option prefix for negation is "no", so
overriding compress-force might look like "noforce:zstd:5" on the inode.
So there's a compelling argument to put force first.

That said, I think compatibility is more important.  On an old kernel,
a file with a force prefix won't be compressed at all if the user
had set up inode attributes without mounting with a compress= option.
With a force suffix, an old kernel still gets some compression, and
also avoids possibly getting the wrong type of compression, like the
slower zlib algorithm instead of zstd.

The prefix form breaks a convention that has been present since the
attributes were introduced.  Compression type is the first argument,
which has been there since the beginning.  The next part of the string
is the level. Fedora’s documentation, among others, already describes
it that way. Those docs were wrong, but people have read them,
and some users have already filled in those strings believing they
have an effect.  Extensions using the same attribute name should go
after the parts of the string that are already defined and in use.
Those constraints together prefer "zstd:3:force" and "zstd:3:noforce"
over "force:zstd:3" and "noforce:zstd:3".

"none" rears its head here:  it uses an exact match, so "none:extension"
is not currently accepted by kernels, and "none:noforce" would mean
"use mount option compression" on an old kernel.  So that's a point in
favor of a separate attribute.

> > 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.

I agree with rejecting "force:none" (or "none:force"--I'll use the
prefix form here to match the text I'm replying to), although that
agreement is conditional on using a single string attribute (see
alternatives below).

"noforce:none" would mean "don't compress this file, even if
compress-force is set in the mount options."  A user might expect
"force:none" to mean "behave like compress-force with chattr +m"
(i.e. exactly the same as noforce:none), but the current compress-force
mount option behavior doesn't honor chattr +m, and "force:none" would
have to replicate what is arguably a bug to be consistent with what the
existing flags and options do.

> 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.

If the all-in-one-attribute-string path is taken, now is the time to
lock down whether these strings have a fixed order, or if it's a set of
strings that can be present or absent.  e.g. is "3:force:zstd" valid?
If there's a compression algorithm called "shiny" with a flag "fast", is
"shiny:7:fast:force" valid, or only "shiny:7:force:fast", or do we make
every compression algorithm squeeze all of its parameters into a single
integer level?  Or will it be a free-for-all, with any order accepted on
input, and all fields optional, so "noforce" by itself is a valid string,
and a user can write "7:force:zstd" or even "5"?

A separate attribute avoids the parsing problem, but trades it for
an ordering problem.

"btrfs.force-compression" could be "yes", "no", or unset (for "look up
the mount option").  A new attribute gets a new name and doesn't touch
the old string parser.  Each of the force strings means something to
a kernel that knows the attribute name.  They are ignored by a kernel
that doesn't know the attribute name, and old kernels can still use the
attributes they do recognize.  Applications work the same way, using
the attributes they know and ignoring the ones they don't.

But a separate attribute forces us to reconsider combining "force" with
"none".  Order of attribute sets matters if they conflict:  a file with
force-compression=yes and compression=zstd, under a rejection rule, could
not be changed to compression=none by setting only btrfs.compression.
An application written before this patch is accepted would not know it
has to do that, so it may fail after a kernel upgrade.

The better option is to document that "force" combined with "none"
just means the same thing as "noforce" combined with "none", so all
combinations are valid.  That allows two attributes (or more extensions
in future) to be added or changed in any order without having to deal
with surprise -EINVALs, and without all the parsing issues.

For a cautionary tale about rejecting invalid option combinations,
look at what happened with conflicting combinations of chattr +C, +c,
and +m rejected by a patch in 2020 (f37c563bab42 "btrfs: add missing
check for nocow and compression inode flags").  These checks are _still_
broken in 2026, accepting invalid combinations of options while rejecting
valid ones, and requiring multiple steps to transition from one valid
flag combination to another.

> >> 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
> 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-11-12  6:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 13:51 [RFC] btrfs: forcing compression per inode via the compression property? Sterling Windmill
2026-11-09 14:00 ` koraynilay
2026-11-09 14:14   ` Sterling Windmill
2026-11-09 20:25     ` koraynilay
2026-11-10  2:50       ` Sterling Windmill
2026-11-10  9:29         ` koraynilay
2026-11-12  6:15           ` Zygo Blaxell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox