Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: "koraynilay" <koray.fra@gmail.com>
To: "Sterling Windmill" <sterling@ampx.net>,
	<linux-btrfs@vger.kernel.org>, "koraynilay" <koray.fra@gmail.com>
Cc: "David Sterba" <dsterba@suse.com>
Subject: Re: [RFC] btrfs: forcing compression per inode via the compression property?
Date: Tue, 10 Nov 2026 10:29:16 +0100	[thread overview]
Message-ID: <DMRFBAXJM4OA.CV6LZNBPYR7J@gmail.com> (raw)
In-Reply-To: <bebe248b-8eb5-48f3-8dc7-ddaf8e4a09c4@app.fastmail.com>

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

  reply	other threads:[~2026-11-10  9:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-11-12  6:15           ` Zygo Blaxell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DMRFBAXJM4OA.CV6LZNBPYR7J@gmail.com \
    --to=koray.fra@gmail.com \
    --cc=dsterba@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=sterling@ampx.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox