[PATCH 0/4] btrfs: add per-inode compression levels in xattrs

[PATCH 0/4] btrfs: add per-inode compression levels in xattrs

[koraynilay]

Add per-inode compression levels using
btrfs property set /path/to/file compression "algo:level", using the
same syntax as the compress mount option
If set on folders, all new children will inherit the setting, while
already existing children will be unaffected.

This patch series keeps a legacy small "bug": when setting
btrfs.compression, it keeps the mount option level, even if the algo is
different (!!), so with e.g. compress=zstd:15 and btrfs.compression=zlib
the data gets compressed at zlib:9 (because it still gets clamped at the
right range). This way the behaviour of chattr +c is also unchanged.

I also wanna thank Zygo for helping me by explaining stuff and for
noticing this "bug".

koraynilay (4):
  btrfs: export btrfs_match_compress_type(), move it to compression.h
  btrfs: also validate compression levels in
    btrfs_compress_is_valid_type()
  btrfs: add per-inode compression levels in xattrs
  btrfs: support inheritance for per-inode compression levels

 fs/btrfs/btrfs_inode.h |  1 +
 fs/btrfs/compression.c | 28 ++++++++++++++++++---
 fs/btrfs/compression.h |  3 ++-
 fs/btrfs/inode.c       | 14 +++++++++++
 fs/btrfs/props.c       | 57 +++++++++++++++++++++++++++++++++---------
 fs/btrfs/super.c       |  8 ------
 6 files changed, 87 insertions(+), 24 deletions(-)


base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4
-- 
2.55.0

[PATCH 1/4] btrfs: export btrfs_match_compress_type(), move it to compression.h

[koraynilay]

Export btrfs_match_compress_type() from being a static and locally used
function in super.c and move it to compression.c, exporting it in
compression.h. This allows it to be used in the next patches as validation
for compression algorithm names.

Assisted-by: Gemini:3.1-pro antigravity-cli-1.1.5
Signed-off-by: koraynilay <koray.fra@gmail.com>
---
 fs/btrfs/compression.c | 17 +++++++++++++++++
 fs/btrfs/compression.h |  1 +
 fs/btrfs/super.c       |  8 --------
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index ffb6b52863a7..58138f300b58 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1660,3 +1660,20 @@ int btrfs_compress_str2level(unsigned int type, const char *str, int *level_ret)
 	*level_ret = btrfs_compress_set_level(type, level);
 	return 0;
 }
+
+/**
+ * btrfs_match_compress_type - Check if the string matches the compression type
+ * @string:         The string to check
+ * @type:           The compression type string (name) to match against (e.g. "zstd")
+ * @may_have_level: If true, the string may have a level suffix (e.g., ":1")
+ *
+ * Return: %true if the string matches the type and, if %may_have_level is %true,
+ * has a level suffix, %false otherwise.
+ */
+bool btrfs_match_compress_type(const char *string, const char *type, bool may_have_level)
+{
+	const int len = strlen(type);
+
+	return (strncmp(string, type, len) == 0) &&
+		((may_have_level && string[len] == ':') || string[len] == '\0');
+}
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 1022dc53ec51..e67ba47b4cdc 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -97,6 +97,7 @@ void btrfs_submit_compressed_write(struct btrfs_ordered_extent *ordered,
 void btrfs_submit_compressed_read(struct btrfs_bio *bbio);
 
 int btrfs_compress_str2level(unsigned int type, const char *str, int *level_ret);
+bool btrfs_match_compress_type(const char *string, const char *type, bool may_have_level);
 
 struct folio *btrfs_alloc_compr_folio(struct btrfs_fs_info *fs_info, gfp_t gfp);
 void btrfs_free_compr_folio(struct folio *folio);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index f4e34898d581..7a21085c33c5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -264,14 +264,6 @@ static const struct fs_parameter_spec btrfs_fs_parameters[] = {
 	{}
 };
 
-static bool btrfs_match_compress_type(const char *string, const char *type, bool may_have_level)
-{
-	const int len = strlen(type);
-
-	return (strncmp(string, type, len) == 0) &&
-		((may_have_level && string[len] == ':') || string[len] == '\0');
-}
-
 static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
 				const struct fs_parameter *param, int opt)
 {
-- 
2.55.0

[PATCH 2/4] btrfs: also validate compression levels in btrfs_compress_is_valid_type()

[koraynilay]

Change btrfs_compress_is_valid_type() to use btrfs_match_compress_type()
and btrfs_compress_str2level() instead of a simple strncmp, which would
validate even incorrect strings (e.g. "zstd:invalid" or "zstdinvalid").

This also makes the function validate levels in the same way the compress=
option gets parsed, allowing bigger or smaller values, but still clamping
them to the min or max supported.

Furthermore, remove the len parameter, since now it requires a
NUL-terminated string because of btrfs_compress_str2level(); this change is
fine because btrfs_compress_is_valid_type() is used only once in props.c

Assisted-by: Gemini:3.1-pro antigravity-cli-1.1.5
Signed-off-by: koraynilay <koray.fra@gmail.com>
---
 fs/btrfs/compression.c | 11 ++++++++---
 fs/btrfs/compression.h |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 58138f300b58..833c5f45afec 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -70,18 +70,23 @@ static struct compressed_bio *alloc_compressed_bio(struct btrfs_inode *inode,
 	return to_compressed_bio(bbio);
 }
 
-bool btrfs_compress_is_valid_type(const char *str, size_t len)
+bool btrfs_compress_is_valid_type(const char *str)
 {
+	size_t len = strlen(str);
 	int i;
 
 	for (i = 1; i < ARRAY_SIZE(btrfs_compress_types); i++) {
 		size_t comp_len = strlen(btrfs_compress_types[i]);
+		const char *comp_type = btrfs_compress_types[i];
+		int tmp_level;
 
 		if (len < comp_len)
 			continue;
 
-		if (!strncmp(btrfs_compress_types[i], str, comp_len))
-			return true;
+		if (btrfs_match_compress_type(str, comp_type, true)) {
+			if (btrfs_compress_str2level(i, str + comp_len, &tmp_level) == 0)
+				return true;
+		}
 	}
 	return false;
 }
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index e67ba47b4cdc..c63bed9f4152 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -132,7 +132,7 @@ extern const struct btrfs_compress_levels btrfs_lzo_compress;
 extern const struct btrfs_compress_levels btrfs_zstd_compress;
 
 const char* btrfs_compress_type2str(enum btrfs_compression_type type);
-bool btrfs_compress_is_valid_type(const char *str, size_t len);
+bool btrfs_compress_is_valid_type(const char *str);
 
 int btrfs_compress_heuristic(struct btrfs_inode *inode, u64 start, u64 end);
 
-- 
2.55.0

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

[koraynilay]

Add support for specifying algo:level using
btrfs property set /path/to/file compression "algo:level".

Add a signed 8 bit prop_compress_level property to btrfs_inode, which can
support from level -128 to 127, plenty for the currently supported
algo:level(s).

Change prop_compression_apply() to use the already present
btrfs_match_compress_type() and btrfs_compress_str2level() to parse the
xattr, using kmemdup_nul() to convert the user-supplied xattr value to a
NUL-terminated string to be used by btrfs_compress_str2level().
The same approach was taken in prop_compression_validate() for
btrfs_compress_is_valid_type().

Assisted-by: Gemini:3.1-pro antigravity-cli-1.1.5
Signed-off-by: koraynilay <koray.fra@gmail.com>
---
 fs/btrfs/btrfs_inode.h |  1 +
 fs/btrfs/inode.c       | 14 ++++++++++++++
 fs/btrfs/props.c       | 43 +++++++++++++++++++++++++++++++++---------
 3 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 1082fa92c145..0a4e567d0109 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -130,6 +130,7 @@ struct btrfs_inode {
 
 	/* Cached value of inode property 'compression'. */
 	u8 prop_compress;
+	s8 prop_compress_level;
 
 	/*
 	 * Force compression on the file using the defrag ioctl, could be
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2534cd9284d5..b63b5006fd78 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -938,6 +938,13 @@ static void compress_file_range(struct btrfs_work *work)
 		compress_level = inode->defrag_compress_level;
 	} else if (inode->prop_compress) {
 		compress_type = inode->prop_compress;
+		/*
+		 * If level not specified (i.e. is 0), use the fs default. If
+		 * compress_type is different, keep the level anyway and let
+		 * the rest of the code clamp it.
+		 */
+		if (inode->prop_compress_level)
+			compress_level = inode->prop_compress_level;
 	}
 
 	/* Compression level is applied here. */
@@ -2326,6 +2333,13 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
 			compress_level = inode->defrag_compress_level;
 		} else if (inode->prop_compress) {
 			compress_type = inode->prop_compress;
+			/*
+			 * If level not specified (i.e. is 0), use the fs default. If
+			 * compress_type is different, keep the level anyway and let
+			 * the rest of the code clamp it.
+			 */
+			if (inode->prop_compress_level)
+				compress_level = inode->prop_compress_level;
 		}
 		cb = btrfs_compress_bio(inode, 0, blocksize, compress_type, compress_level, 0);
 		if (IS_ERR(cb)) {
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index bb77d46376d4..f79a61a2759f 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -295,20 +295,28 @@ int btrfs_load_inode_props(struct btrfs_inode *inode, struct btrfs_path *path)
 static int prop_compression_validate(const struct btrfs_inode *inode,
 				     const char *value, size_t len)
 {
+	int ret = -EINVAL;
+	const char *value_str;
+
 	if (!btrfs_inode_can_compress(inode))
 		return -EINVAL;
 
 	if (!value)
 		return 0;
 
-	if (btrfs_compress_is_valid_type(value, len))
-		return 0;
-
 	if ((len == 2 && strncmp("no", value, 2) == 0) ||
 	    (len == 4 && strncmp("none", value, 4) == 0))
 		return 0;
 
-	return -EINVAL;
+	value_str = kmemdup_nul(value, len, GFP_KERNEL);
+	if (!value_str)
+		return -ENOMEM;
+
+	if (btrfs_compress_is_valid_type(value_str))
+		ret = 0;
+
+	kfree(value_str);
+	return ret;
 }
 
 static int prop_compression_apply(struct btrfs_inode *inode, const char *value,
@@ -316,6 +324,9 @@ static int prop_compression_apply(struct btrfs_inode *inode, const char *value,
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	int type;
+	int level = 0;
+	int ret = 0;
+	const char *value_str;
 
 	/* Reset to defaults */
 	if (len == 0) {
@@ -335,23 +346,37 @@ static int prop_compression_apply(struct btrfs_inode *inode, const char *value,
 		return 0;
 	}
 
-	if (!strncmp("lzo", value, 3)) {
+	value_str = kmemdup_nul(value, len, GFP_KERNEL);
+	if (!value_str)
+		return -ENOMEM;
+
+	if (btrfs_match_compress_type(value_str, "lzo", true)) {
 		type = BTRFS_COMPRESS_LZO;
 		btrfs_set_fs_incompat(fs_info, COMPRESS_LZO);
-	} else if (!strncmp("zlib", value, 4)) {
+	} else if (btrfs_match_compress_type(value_str, "zlib", true)) {
 		type = BTRFS_COMPRESS_ZLIB;
-	} else if (!strncmp("zstd", value, 4)) {
+		ret = btrfs_compress_str2level(type, value_str + 4, &level);
+		if (ret < 0)
+			goto out;
+	} else if (btrfs_match_compress_type(value_str, "zstd", true)) {
 		type = BTRFS_COMPRESS_ZSTD;
+		ret = btrfs_compress_str2level(type, value_str + 4, &level);
+		if (ret < 0)
+			goto out;
 		btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD);
 	} else {
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	inode->flags &= ~BTRFS_INODE_NOCOMPRESS;
 	inode->flags |= BTRFS_INODE_COMPRESS;
 	inode->prop_compress = type;
+	inode->prop_compress_level = level;
 
-	return 0;
+out:
+	kfree(value_str);
+	return ret;
 }
 
 static bool prop_compression_ignore(const struct btrfs_inode *inode)
-- 
2.55.0

[PATCH 4/4] btrfs: support inheritance for per-inode compression levels

[koraynilay]

Change prop_handler's extract() signature to take an output buffer and its
length. This allows for the algo:level string to be generated dynamically,
but now the caller is in charge of managing that buffer memory. Before this
patch prop_compression_extract() would return only the address of
statically compiled string without being able to add the level, now it
concatenates the level to that statically compiled string.

This signature change is fine because the only currently supported prop is
compression, so there is only 1 extract() function.

Assisted-by: Gemini:3.1-pro antigravity-cli-1.1.5
Signed-off-by: koraynilay <koray.fra@gmail.com>
---
 fs/btrfs/props.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index f79a61a2759f..9c97792800fb 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -27,7 +27,7 @@ struct prop_handler {
 	int (*validate)(const struct btrfs_inode *inode, const char *value,
 			size_t len);
 	int (*apply)(struct btrfs_inode *inode, const char *value, size_t len);
-	const char *(*extract)(const struct btrfs_inode *inode);
+	const char *(*extract)(const struct btrfs_inode *inode, char *buf, size_t len);
 	bool (*ignore)(const struct btrfs_inode *inode);
 	int inheritable;
 };
@@ -395,12 +395,19 @@ static bool prop_compression_ignore(const struct btrfs_inode *inode)
 	return false;
 }
 
-static const char *prop_compression_extract(const struct btrfs_inode *inode)
+static const char *prop_compression_extract(const struct btrfs_inode *inode,
+					    char *buf, size_t len)
 {
 	switch (inode->prop_compress) {
 	case BTRFS_COMPRESS_ZLIB:
 	case BTRFS_COMPRESS_LZO:
 	case BTRFS_COMPRESS_ZSTD:
+		if (inode->prop_compress_level) {
+			snprintf(buf, len, "%s:%d",
+				 btrfs_compress_type2str(inode->prop_compress),
+				 inode->prop_compress_level);
+			return buf;
+		}
 		return btrfs_compress_type2str(inode->prop_compress);
 	default:
 		break;
@@ -437,6 +444,7 @@ int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
 		const struct prop_handler *h = &prop_handlers[i];
 		const char *value;
 		u64 num_bytes = 0;
+		char buf[16];
 
 		if (!h->inheritable)
 			continue;
@@ -444,7 +452,7 @@ int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
 		if (h->ignore(inode))
 			continue;
 
-		value = h->extract(parent);
+		value = h->extract(parent, buf, sizeof(buf));
 		if (!value)
 			continue;
 
-- 
2.55.0

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

[Qu Wenruo]

在 2026/8/8 12:04, koraynilay 写道:
> Add per-inode compression levels using
> btrfs property set /path/to/file compression "algo:level", using the
> same syntax as the compress mount option
> If set on folders, all new children will inherit the setting, while
> already existing children will be unaffected.
> 
> This patch series keeps a legacy small "bug": when setting
> btrfs.compression, it keeps the mount option level, even if the algo is
> different (!!), so with e.g. compress=zstd:15 and btrfs.compression=zlib
> the data gets compressed at zlib:9 (because it still gets clamped at the
> right range). This way the behaviour of chattr +c is also unchanged.

I'm not sure if this is the correct behavior in the first place.

As you already mentioned, zstd and zlib have very different compression 
level range, using the incorrect level makes no sense (and it's being 
clamped anyway).

I think we should go the default level when not specified, which makes 
more sense, and that would definitely be something worth fixing.

Thanks,
Qu

> 
> I also wanna thank Zygo for helping me by explaining stuff and for
> noticing this "bug".
> 
> koraynilay (4):
>    btrfs: export btrfs_match_compress_type(), move it to compression.h
>    btrfs: also validate compression levels in
>      btrfs_compress_is_valid_type()
>    btrfs: add per-inode compression levels in xattrs
>    btrfs: support inheritance for per-inode compression levels
> 
>   fs/btrfs/btrfs_inode.h |  1 +
>   fs/btrfs/compression.c | 28 ++++++++++++++++++---
>   fs/btrfs/compression.h |  3 ++-
>   fs/btrfs/inode.c       | 14 +++++++++++
>   fs/btrfs/props.c       | 57 +++++++++++++++++++++++++++++++++---------
>   fs/btrfs/super.c       |  8 ------
>   6 files changed, 87 insertions(+), 24 deletions(-)
> 
> 
> base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4

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

[koraynilay]

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

On Sun Aug 9, 2026 at 2:17 AM CEST, Qu Wenruo wrote:
> I'm not sure if this is the correct behavior in the first place.
>
> As you already mentioned, zstd and zlib have very different compression 
> level range, using the incorrect level makes no sense (and it's being 
> clamped anyway).
>
> I think we should go the default level when not specified, which makes 
> more sense, and that would definitely be something worth fixing.

Yes, I also think that would be best, but my main concern would be it
changing how chattr +c behaves (I'm less concerned about the btrfs prop
set file compression "zstd" case, since IMO that implies the user wants
the default level).

The options I considered were:
1) keep the "bug", like I did for now;
2) keep the "bug", but only if the compress= algo is the same
   as the btrfs.compression one, if they aren't, use the default for the
   btrfs.compression algo (e.g. compress=zstd:15 and btrfs.compression=zlib
   would compress the extent at zlib:3 instead of clamp(zlib, 15) = 9)
   (suggested by Zygo);
3) fix the "bug" entirely, which is what I actually accidentally did at
   first, by just setting compress_level = inode->prop_compress_level
   without any check prior to that (which means that by default it would
   use algo:0).

Option 2) is probably the best compromise between breaking existing
scripts and the behaviour making sense, plus it shouldn't change the
chattr +c behaviour, since btrfs takes the algorithm from compress=.

Thanks.

Best,
koraynilay

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

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

[Qu Wenruo]

在 2026/8/9 10:05, koraynilay 写道:
> On Sun Aug 9, 2026 at 2:17 AM CEST, Qu Wenruo wrote:
>> I'm not sure if this is the correct behavior in the first place.
>>
>> As you already mentioned, zstd and zlib have very different compression
>> level range, using the incorrect level makes no sense (and it's being
>> clamped anyway).
>>
>> I think we should go the default level when not specified, which makes
>> more sense, and that would definitely be something worth fixing.
> 
> Yes, I also think that would be best, but my main concern would be it
> changing how chattr +c behaves (I'm less concerned about the btrfs prop
> set file compression "zstd" case, since IMO that implies the user wants
> the default level).

Mind to explain more about the "chattr +c" problem?

IIRC "chattr +c" just set the btrfs.compression XATTR to the default 
zlib if no mount option is specified.

In that case it should be no difference compared to any existing XATTR 
based compression setting.

Thus it's just the same missing level handling, and IMHO since XATTR 
compression level is never specified in XATTR, then the behavior is 
never fully determined, and users should not depend on it.

Even if we changed the behavior to option 3, it should not be a super 
huge user affecting change.
In the end, it's just compression level, affecting compression ratio and 
speed, not really a huge behavior change.

> 
> The options I considered were:
> 1) keep the "bug", like I did for now;
> 2) keep the "bug", but only if the compress= algo is the same
>     as the btrfs.compression one, if they aren't, use the default for the
>     btrfs.compression algo (e.g. compress=zstd:15 and btrfs.compression=zlib
>     would compress the extent at zlib:3 instead of clamp(zlib, 15) = 9)
>     (suggested by Zygo);
> 3) fix the "bug" entirely, which is what I actually accidentally did at
>     first, by just setting compress_level = inode->prop_compress_level
>     without any check prior to that (which means that by default it would
>     use algo:0).

IHMO both option 2 and 3 are acceptable.


The only extra concern is, if we have a new level field in XATTR, can 
older kernels handle it?

And thankfully the existing prop apply handler is checking only the 
first several bytes for different algos, thus the existing code should 
handle the extra appended ":<level>" correctly by just ignoring the level.

So either option 2 or 3 would be fine to me. Although I personally 
prefer option 3 a little more, just because it's much cleaner code wise.

Thanks,
Qu

> 
> Option 2) is probably the best compromise between breaking existing
> scripts and the behaviour making sense, plus it shouldn't change the
> chattr +c behaviour, since btrfs takes the algorithm from compress=.
> 
> Thanks.
> 
> Best,
> koraynilay

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

[koraynilay]

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

> Mind to explain more about the "chattr +c" problem?
>
> IIRC "chattr +c" just set the btrfs.compression XATTR to the default 
> zlib if no mount option is specified.

Hmm, I thought it as "I want to explicitely say to btrfs to compress
this file using whatever optins I set for the fs", e.g. in a situation
where a file was created without compress=, then compress= is used...but
that would be the exact same, as new data would be compressed all the
same because of compress=.
Another (very edge) use case I thought about was "file gets marked as
NOCOMPRESS, then gets truncated and compressible data gets written to it
so I want it to compress again" (since +c resets NOCOPRESS)...but I
hadn't tested it. Now that I did, I realized in that case it probably
wouldn't get past the compression heuristics so it wouldn't be marked as
NOCOMPRESS anyway.

> The only extra concern is, if we have a new level field in XATTR, can 
> older kernels handle it?
>
> And thankfully the existing prop apply handler is checking only the 
> first several bytes for different algos, thus the existing code should 
> handle the extra appended ":<level>" correctly by just ignoring the level.

It should be fine, as the current code also accepts "zlibabcd",
"zlib:invalid" and so on; it gets applied successfully with btrfs prop
set and gets rightly compressed using "zlib" (well, using the level from
the mount point because of the "bug", but yeah).

> So either option 2 or 3 would be fine to me. Although I personally 
> prefer option 3 a little more, just because it's much cleaner code wise.

I'll change it back to the simple assignment and send v2 of the patch
series then.

Thanks again.

Best,
koraynilay

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

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

[Zygo Blaxell]

On Sun, Aug 09, 2026 at 10:30:45AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/8/9 10:05, koraynilay 写道:
> > On Sun Aug 9, 2026 at 2:17 AM CEST, Qu Wenruo wrote:
> > > I'm not sure if this is the correct behavior in the first place.
> > > 
> > > As you already mentioned, zstd and zlib have very different compression
> > > level range, using the incorrect level makes no sense (and it's being
> > > clamped anyway).
> > > 
> > > I think we should go the default level when not specified, which makes
> > > more sense, and that would definitely be something worth fixing.
> > 
> > Yes, I also think that would be best, but my main concern would be it
> > changing how chattr +c behaves (I'm less concerned about the btrfs prop
> > set file compression "zstd" case, since IMO that implies the user wants
> > the default level).
> 
> Mind to explain more about the "chattr +c" problem?
> 
> IIRC "chattr +c" just set the btrfs.compression XATTR to the default zlib if
> no mount option is specified.

chattr with _any_ argument resets the btrfs.compression xattr _every_
time, even if the caller changes a flag something that doesn't look like
it would affect compression, e.g. chattr +A or +d.

That can lead to surprising results, like changing compress type from
zstd to zlib, if the original btrfs.compression attribute doesn't match
the mount option.

> In that case it should be no difference compared to any existing XATTR based
> compression setting.
> 
> Thus it's just the same missing level handling, and IMHO since XATTR
> compression level is never specified in XATTR, then the behavior is never
> fully determined, and users should not depend on it.
> 
> Even if we changed the behavior to option 3, it should not be a super huge
> user affecting change.
> In the end, it's just compression level, affecting compression ratio and
> speed, not really a huge behavior change.
> 
> > 
> > The options I considered were:
> > 1) keep the "bug", like I did for now;
> > 2) keep the "bug", but only if the compress= algo is the same
> >     as the btrfs.compression one, if they aren't, use the default for the
> >     btrfs.compression algo (e.g. compress=zstd:15 and btrfs.compression=zlib
> >     would compress the extent at zlib:3 instead of clamp(zlib, 15) = 9)
> >     (suggested by Zygo);
> > 3) fix the "bug" entirely, which is what I actually accidentally did at
> >     first, by just setting compress_level = inode->prop_compress_level
> >     without any check prior to that (which means that by default it would
> >     use algo:0).
> 
> IHMO both option 2 and 3 are acceptable.
> 
> 
> The only extra concern is, if we have a new level field in XATTR, can older
> kernels handle it?
> 
> And thankfully the existing prop apply handler is checking only the first
> several bytes for different algos, thus the existing code should handle the
> extra appended ":<level>" correctly by just ignoring the level.
> 
> So either option 2 or 3 would be fine to me. Although I personally prefer
> option 3 a little more, just because it's much cleaner code wise.

Option 2 preserves legacy behavior that is 12 years old now, and it
costs a single comparison in two 'if' statements.

Option 3 makes an already confusing situation worse--it makes the
underspecified behavior change depending on kernel version.

> Thanks,
> Qu
> 
> > 
> > Option 2) is probably the best compromise between breaking existing
> > scripts and the behaviour making sense, plus it shouldn't change the
> > chattr +c behaviour, since btrfs takes the algorithm from compress=.
> > 
> > Thanks.
> > 
> > Best,
> > koraynilay
> 
> 

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

[Qu Wenruo]

在 2026/8/10 08:47, Zygo Blaxell 写道:
> On Sun, Aug 09, 2026 at 10:30:45AM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2026/8/9 10:05, koraynilay 写道:
>>> On Sun Aug 9, 2026 at 2:17 AM CEST, Qu Wenruo wrote:
>>>> I'm not sure if this is the correct behavior in the first place.
>>>>
>>>> As you already mentioned, zstd and zlib have very different compression
>>>> level range, using the incorrect level makes no sense (and it's being
>>>> clamped anyway).
>>>>
>>>> I think we should go the default level when not specified, which makes
>>>> more sense, and that would definitely be something worth fixing.
>>>
>>> Yes, I also think that would be best, but my main concern would be it
>>> changing how chattr +c behaves (I'm less concerned about the btrfs prop
>>> set file compression "zstd" case, since IMO that implies the user wants
>>> the default level).
>>
>> Mind to explain more about the "chattr +c" problem?
>>
>> IIRC "chattr +c" just set the btrfs.compression XATTR to the default zlib if
>> no mount option is specified.
> 
> chattr with _any_ argument resets the btrfs.compression xattr _every_
> time, even if the caller changes a flag something that doesn't look like
> it would affect compression, e.g. chattr +A or +d.

This is the known problem of chattr.

> 
> That can lead to surprising results, like changing compress type from
> zstd to zlib, if the original btrfs.compression attribute doesn't match
> the mount option.

This doesn't only apply to c, but all other chattr options.

That's why proper chattr should always include the full attrs, not only 
the one to change.

> 
>> In that case it should be no difference compared to any existing XATTR based
>> compression setting.
>>
>> Thus it's just the same missing level handling, and IMHO since XATTR
>> compression level is never specified in XATTR, then the behavior is never
>> fully determined, and users should not depend on it.
>>
>> Even if we changed the behavior to option 3, it should not be a super huge
>> user affecting change.
>> In the end, it's just compression level, affecting compression ratio and
>> speed, not really a huge behavior change.
>>
>>>
>>> The options I considered were:
>>> 1) keep the "bug", like I did for now;
>>> 2) keep the "bug", but only if the compress= algo is the same
>>>      as the btrfs.compression one, if they aren't, use the default for the
>>>      btrfs.compression algo (e.g. compress=zstd:15 and btrfs.compression=zlib
>>>      would compress the extent at zlib:3 instead of clamp(zlib, 15) = 9)
>>>      (suggested by Zygo);
>>> 3) fix the "bug" entirely, which is what I actually accidentally did at
>>>      first, by just setting compress_level = inode->prop_compress_level
>>>      without any check prior to that (which means that by default it would
>>>      use algo:0).
>>
>> IHMO both option 2 and 3 are acceptable.
>>
>>
>> The only extra concern is, if we have a new level field in XATTR, can older
>> kernels handle it?
>>
>> And thankfully the existing prop apply handler is checking only the first
>> several bytes for different algos, thus the existing code should handle the
>> extra appended ":<level>" correctly by just ignoring the level.
>>
>> So either option 2 or 3 would be fine to me. Although I personally prefer
>> option 3 a little more, just because it's much cleaner code wise.
> 
> Option 2 preserves legacy behavior that is 12 years old now, and it
> costs a single comparison in two 'if' statements.
> 
> Option 3 makes an already confusing situation worse--it makes the
> underspecified behavior change depending on kernel version.

One should never rely on something not documented in the first place.

We're to make it more clear, not making some undocumented BS to be the 
common case.

> 
>> Thanks,
>> Qu
>>
>>>
>>> Option 2) is probably the best compromise between breaking existing
>>> scripts and the behaviour making sense, plus it shouldn't change the
>>> chattr +c behaviour, since btrfs takes the algorithm from compress=.
>>>
>>> Thanks.
>>>
>>> Best,
>>> koraynilay
>>
>>

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

[Zygo Blaxell]

On Mon, Aug 10, 2026 at 08:50:45AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/8/10 08:47, Zygo Blaxell 写道:
> > On Sun, Aug 09, 2026 at 10:30:45AM +0930, Qu Wenruo wrote:
> > > 
> > > 
> > > 在 2026/8/9 10:05, koraynilay 写道:
> > > > On Sun Aug 9, 2026 at 2:17 AM CEST, Qu Wenruo wrote:
> > > > > I'm not sure if this is the correct behavior in the first place.
> > > > > 
> > > > > As you already mentioned, zstd and zlib have very different compression
> > > > > level range, using the incorrect level makes no sense (and it's being
> > > > > clamped anyway).
> > > > > 
> > > > > I think we should go the default level when not specified, which makes
> > > > > more sense, and that would definitely be something worth fixing.
> > > > 
> > > > Yes, I also think that would be best, but my main concern would be it
> > > > changing how chattr +c behaves (I'm less concerned about the btrfs prop
> > > > set file compression "zstd" case, since IMO that implies the user wants
> > > > the default level).
> > > 
> > > Mind to explain more about the "chattr +c" problem?
> > > 
> > > IIRC "chattr +c" just set the btrfs.compression XATTR to the default zlib if
> > > no mount option is specified.
> > 
> > chattr with _any_ argument resets the btrfs.compression xattr _every_
> > time, even if the caller changes a flag something that doesn't look like
> > it would affect compression, e.g. chattr +A or +d.
> 
> This is the known problem of chattr.
> 
> > 
> > That can lead to surprising results, like changing compress type from
> > zstd to zlib, if the original btrfs.compression attribute doesn't match
> > the mount option.
> 
> This doesn't only apply to c, but all other chattr options.
> 
> That's why proper chattr should always include the full attrs, not only the
> one to change.
> 
> > 
> > > In that case it should be no difference compared to any existing XATTR based
> > > compression setting.
> > > 
> > > Thus it's just the same missing level handling, and IMHO since XATTR
> > > compression level is never specified in XATTR, then the behavior is never
> > > fully determined, and users should not depend on it.
> > > 
> > > Even if we changed the behavior to option 3, it should not be a super huge
> > > user affecting change.
> > > In the end, it's just compression level, affecting compression ratio and
> > > speed, not really a huge behavior change.
> > > 
> > > > 
> > > > The options I considered were:
> > > > 1) keep the "bug", like I did for now;
> > > > 2) keep the "bug", but only if the compress= algo is the same
> > > >      as the btrfs.compression one, if they aren't, use the default for the
> > > >      btrfs.compression algo (e.g. compress=zstd:15 and btrfs.compression=zlib
> > > >      would compress the extent at zlib:3 instead of clamp(zlib, 15) = 9)
> > > >      (suggested by Zygo);
> > > > 3) fix the "bug" entirely, which is what I actually accidentally did at
> > > >      first, by just setting compress_level = inode->prop_compress_level
> > > >      without any check prior to that (which means that by default it would
> > > >      use algo:0).
> > > 
> > > IHMO both option 2 and 3 are acceptable.
> > > 
> > > 
> > > The only extra concern is, if we have a new level field in XATTR, can older
> > > kernels handle it?
> > > 
> > > And thankfully the existing prop apply handler is checking only the first
> > > several bytes for different algos, thus the existing code should handle the
> > > extra appended ":<level>" correctly by just ignoring the level.
> > > 
> > > So either option 2 or 3 would be fine to me. Although I personally prefer
> > > option 3 a little more, just because it's much cleaner code wise.
> > 
> > Option 2 preserves legacy behavior that is 12 years old now, and it
> > costs a single comparison in two 'if' statements.
> > 
> > Option 3 makes an already confusing situation worse--it makes the
> > underspecified behavior change depending on kernel version.
> 
> One should never rely on something not documented in the first place.

Option 3 prevents existing mount-option compression level specifications
from working when the attribute agress with the mount option; otherwise,
they would be blocked by a btrfs.compression string that doesn't specify
a level.  That's a _regression_.

The current behavior is documented all over the place, used in scripts,
etc.  The top two from Google:

    https://wiki.tnonline.net/w/Btrfs/Compression
    https://fedoraproject.org/wiki/Changes/BtrfsTransparentCompression#Q:_If_I_use_'btrfs_property'_to_set_compression,_what_level_is_used_for_zstd?

followed by dozens of posts on forums and reddit, as users discover
how it works by experimentation or reading the kernel code.

The Fedora link is interesting because it describes two opposite
behaviors: once in prose and once in a code example (the code example
is correct, the prose is wrong).

One notably missing reference is the btrfs documentation itself:

    https://btrfs.readthedocs.io/en/latest/ch-mount-options.html
    https://btrfs.readthedocs.io/en/latest/Compression.html

Neither of these completely describe the current behavior, other than
to say that if the level is specified in btrfs.compression, it will be
implemented one day.  That statement is not inconsistent with option 2,
since attributes with level specifications don't need to consider any
of the options, and the statement doesn't say what happens when the
level isn't specified in the attribute.

Documented or not, it's been _established_ behavior for over a decade.
At this point, documentation that contradicts the status quo is wrong,
and should be corrected--as long as the status quo makes sense, which
it currently does.

There does seem to be a use case for having "zstd" continuing to
mean "defer to the mount option for level"--otherwise, to be able to
dynamically change the compress level (e.g. to reduce it under load, and
raise it during maintenance windows), one has to change every inode's
level individually.  This has obvious problems with snapshots that the
current arrangement doesn't have.

> We're to make it more clear, not making some undocumented BS to be the
> common case.
> 
> > 
> > > Thanks,
> > > Qu
> > > 
> > > > 
> > > > Option 2) is probably the best compromise between breaking existing
> > > > scripts and the behaviour making sense, plus it shouldn't change the
> > > > chattr +c behaviour, since btrfs takes the algorithm from compress=.
> > > > 
> > > > Thanks.
> > > > 
> > > > Best,
> > > > koraynilay
> > > 
> > > 
> 
> 

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

[Qu Wenruo]

在 2026/8/10 09:58, Zygo Blaxell 写道:
> On Mon, Aug 10, 2026 at 08:50:45AM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2026/8/10 08:47, Zygo Blaxell 写道:
>>> On Sun, Aug 09, 2026 at 10:30:45AM +0930, Qu Wenruo wrote:
>>>>
>>>>
>>>> 在 2026/8/9 10:05, koraynilay 写道:
>>>>> On Sun Aug 9, 2026 at 2:17 AM CEST, Qu Wenruo wrote:
>>>>>> I'm not sure if this is the correct behavior in the first place.
>>>>>>
>>>>>> As you already mentioned, zstd and zlib have very different compression
>>>>>> level range, using the incorrect level makes no sense (and it's being
>>>>>> clamped anyway).
>>>>>>
>>>>>> I think we should go the default level when not specified, which makes
>>>>>> more sense, and that would definitely be something worth fixing.
>>>>>
>>>>> Yes, I also think that would be best, but my main concern would be it
>>>>> changing how chattr +c behaves (I'm less concerned about the btrfs prop
>>>>> set file compression "zstd" case, since IMO that implies the user wants
>>>>> the default level).
>>>>
>>>> Mind to explain more about the "chattr +c" problem?
>>>>
>>>> IIRC "chattr +c" just set the btrfs.compression XATTR to the default zlib if
>>>> no mount option is specified.
>>>
>>> chattr with _any_ argument resets the btrfs.compression xattr _every_
>>> time, even if the caller changes a flag something that doesn't look like
>>> it would affect compression, e.g. chattr +A or +d.
>>
>> This is the known problem of chattr.
>>
>>>
>>> That can lead to surprising results, like changing compress type from
>>> zstd to zlib, if the original btrfs.compression attribute doesn't match
>>> the mount option.
>>
>> This doesn't only apply to c, but all other chattr options.
>>
>> That's why proper chattr should always include the full attrs, not only the
>> one to change.
>>
>>>
>>>> In that case it should be no difference compared to any existing XATTR based
>>>> compression setting.
>>>>
>>>> Thus it's just the same missing level handling, and IMHO since XATTR
>>>> compression level is never specified in XATTR, then the behavior is never
>>>> fully determined, and users should not depend on it.
>>>>
>>>> Even if we changed the behavior to option 3, it should not be a super huge
>>>> user affecting change.
>>>> In the end, it's just compression level, affecting compression ratio and
>>>> speed, not really a huge behavior change.
>>>>
>>>>>
>>>>> The options I considered were:
>>>>> 1) keep the "bug", like I did for now;
>>>>> 2) keep the "bug", but only if the compress= algo is the same
>>>>>       as the btrfs.compression one, if they aren't, use the default for the
>>>>>       btrfs.compression algo (e.g. compress=zstd:15 and btrfs.compression=zlib
>>>>>       would compress the extent at zlib:3 instead of clamp(zlib, 15) = 9)
>>>>>       (suggested by Zygo);
>>>>> 3) fix the "bug" entirely, which is what I actually accidentally did at
>>>>>       first, by just setting compress_level = inode->prop_compress_level
>>>>>       without any check prior to that (which means that by default it would
>>>>>       use algo:0).
>>>>
>>>> IHMO both option 2 and 3 are acceptable.
>>>>
>>>>
>>>> The only extra concern is, if we have a new level field in XATTR, can older
>>>> kernels handle it?
>>>>
>>>> And thankfully the existing prop apply handler is checking only the first
>>>> several bytes for different algos, thus the existing code should handle the
>>>> extra appended ":<level>" correctly by just ignoring the level.
>>>>
>>>> So either option 2 or 3 would be fine to me. Although I personally prefer
>>>> option 3 a little more, just because it's much cleaner code wise.
>>>
>>> Option 2 preserves legacy behavior that is 12 years old now, and it
>>> costs a single comparison in two 'if' statements.
>>>
>>> Option 3 makes an already confusing situation worse--it makes the
>>> underspecified behavior change depending on kernel version.
>>
>> One should never rely on something not documented in the first place.
> 
> Option 3 prevents existing mount-option compression level specifications
> from working when the attribute agress with the mount option; otherwise,
> they would be blocked by a btrfs.compression string that doesn't specify
> a level.  That's a _regression_.

Let me be this clear, the current one nor option 2 is not working either.

If the current algo is different from the XATTR algo, it will be 
whatever random number clamped to the XATTR algo for the current code.

This applies to the option 2 solution. When mount option changed, the 
level will suddenly change from whatever previous mount option to the 
default.

It's always broken no matter whatever, I do not want to waste time to 
use another broken behavior to mask a more broken one.

> 
> The current behavior is documented all over the place, used in scripts,
> etc.  The top two from Google:
> 
>      https://wiki.tnonline.net/w/Btrfs/Compression
>      https://fedoraproject.org/wiki/Changes/BtrfsTransparentCompression#Q:_If_I_use_'btrfs_property'_to_set_compression,_what_level_is_used_for_zstd?

None of them is the official btrfs-progs man page.

The first URL doesn't even resolve here.

I don't want to repeat myself, all those confusion is from the bug itself.

Instead of building everything upon an uncertain basis, do it correctly.

And I have repeated it several times, even the behavior change is 
minimal only affecting the level.

I see no point why you treat broken behavior so dogmatically.

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

[Zygo Blaxell]

On Mon, Aug 10, 2026 at 10:21:11AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/8/10 09:58, Zygo Blaxell 写道:
> > On Mon, Aug 10, 2026 at 08:50:45AM +0930, Qu Wenruo wrote:
> > > 
> > > 
> > > 在 2026/8/10 08:47, Zygo Blaxell 写道:
> > > > On Sun, Aug 09, 2026 at 10:30:45AM +0930, Qu Wenruo wrote:
> > > > > 
> > > > > 
> > > > > 在 2026/8/9 10:05, koraynilay 写道:
> > > > > > On Sun Aug 9, 2026 at 2:17 AM CEST, Qu Wenruo wrote:
> > > > > > > I'm not sure if this is the correct behavior in the first place.
> > > > > > > 
> > > > > > > As you already mentioned, zstd and zlib have very different compression
> > > > > > > level range, using the incorrect level makes no sense (and it's being
> > > > > > > clamped anyway).
> > > > > > > 
> > > > > > > I think we should go the default level when not specified, which makes
> > > > > > > more sense, and that would definitely be something worth fixing.
> > > > > > 
> > > > > > Yes, I also think that would be best, but my main concern would be it
> > > > > > changing how chattr +c behaves (I'm less concerned about the btrfs prop
> > > > > > set file compression "zstd" case, since IMO that implies the user wants
> > > > > > the default level).
> > > > > 
> > > > > Mind to explain more about the "chattr +c" problem?
> > > > > 
> > > > > IIRC "chattr +c" just set the btrfs.compression XATTR to the default zlib if
> > > > > no mount option is specified.
> > > > 
> > > > chattr with _any_ argument resets the btrfs.compression xattr _every_
> > > > time, even if the caller changes a flag something that doesn't look like
> > > > it would affect compression, e.g. chattr +A or +d.
> > > 
> > > This is the known problem of chattr.
> > > 
> > > > 
> > > > That can lead to surprising results, like changing compress type from
> > > > zstd to zlib, if the original btrfs.compression attribute doesn't match
> > > > the mount option.
> > > 
> > > This doesn't only apply to c, but all other chattr options.
> > > 
> > > That's why proper chattr should always include the full attrs, not only the
> > > one to change.
> > > 
> > > > 
> > > > > In that case it should be no difference compared to any existing XATTR based
> > > > > compression setting.
> > > > > 
> > > > > Thus it's just the same missing level handling, and IMHO since XATTR
> > > > > compression level is never specified in XATTR, then the behavior is never
> > > > > fully determined, and users should not depend on it.
> > > > > 
> > > > > Even if we changed the behavior to option 3, it should not be a super huge
> > > > > user affecting change.
> > > > > In the end, it's just compression level, affecting compression ratio and
> > > > > speed, not really a huge behavior change.
> > > > > 
> > > > > > 
> > > > > > The options I considered were:
> > > > > > 1) keep the "bug", like I did for now;
> > > > > > 2) keep the "bug", but only if the compress= algo is the same
> > > > > >       as the btrfs.compression one, if they aren't, use the default for the
> > > > > >       btrfs.compression algo (e.g. compress=zstd:15 and btrfs.compression=zlib
> > > > > >       would compress the extent at zlib:3 instead of clamp(zlib, 15) = 9)
> > > > > >       (suggested by Zygo);
> > > > > > 3) fix the "bug" entirely, which is what I actually accidentally did at
> > > > > >       first, by just setting compress_level = inode->prop_compress_level
> > > > > >       without any check prior to that (which means that by default it would
> > > > > >       use algo:0).
> > > > > 
> > > > > IHMO both option 2 and 3 are acceptable.
> > > > > 
> > > > > 
> > > > > The only extra concern is, if we have a new level field in XATTR, can older
> > > > > kernels handle it?
> > > > > 
> > > > > And thankfully the existing prop apply handler is checking only the first
> > > > > several bytes for different algos, thus the existing code should handle the
> > > > > extra appended ":<level>" correctly by just ignoring the level.
> > > > > 
> > > > > So either option 2 or 3 would be fine to me. Although I personally prefer
> > > > > option 3 a little more, just because it's much cleaner code wise.
> > > > 
> > > > Option 2 preserves legacy behavior that is 12 years old now, and it
> > > > costs a single comparison in two 'if' statements.
> > > > 
> > > > Option 3 makes an already confusing situation worse--it makes the
> > > > underspecified behavior change depending on kernel version.
> > > 
> > > One should never rely on something not documented in the first place.
> > 
> > Option 3 prevents existing mount-option compression level specifications
> > from working when the attribute agress with the mount option; otherwise,
> > they would be blocked by a btrfs.compression string that doesn't specify
> > a level.  That's a _regression_.
> 
> Let me be this clear, the current one nor option 2 is not working either.
> 
> If the current algo is different from the XATTR algo, it will be whatever
> random number clamped to the XATTR algo for the current code.
> 
> This applies to the option 2 solution. When mount option changed, the level
> will suddenly change from whatever previous mount option to the default.
> 
> It's always broken no matter whatever, I do not want to waste time to use
> another broken behavior to mask a more broken one.

It's not broken during the time when the mount option is the same as
the xattr, and that case seems to be the most common one.  Users don't
change the compression type on the fly or per inode--they use zstd all
the time, and only change the level when it matters.

This stops working with the option 3 proposal (or more specifically, with
my comment on the other patch about when to set compress_level to 0):
it always sets the level to the default, and ignores the mount option,
even when that would make sense and be the intended result.

> > The current behavior is documented all over the place, used in scripts,
> > etc.  The top two from Google:
> > 
> >      https://wiki.tnonline.net/w/Btrfs/Compression
> >      https://fedoraproject.org/wiki/Changes/BtrfsTransparentCompression#Q:_If_I_use_'btrfs_property'_to_set_compression,_what_level_is_used_for_zstd?
> 
> None of them is the official btrfs-progs man page.
> 
> The first URL doesn't even resolve here.
> 
> I don't want to repeat myself, all those confusion is from the bug itself.
> 
> Instead of building everything upon an uncertain basis, do it correctly.
> 
> And I have repeated it several times, even the behavior change is minimal
> only affecting the level.
> 
> I see no point why you treat broken behavior so dogmatically.

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

[koraynilay]

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

On Mon Aug 10, 2026 at 2:51 AM CEST, Qu Wenruo wrote:
>>>>> So either option 2 or 3 would be fine to me. Although I personally prefer
>>>>> option 3 a little more, just because it's much cleaner code wise.
>>>>
>>>> Option 2 preserves legacy behavior that is 12 years old now, and it
>>>> costs a single comparison in two 'if' statements.
>>>>
>>>> Option 3 makes an already confusing situation worse--it makes the
>>>> underspecified behavior change depending on kernel version.
>>>
>>> One should never rely on something not documented in the first place.
>> 
>> Option 3 prevents existing mount-option compression level specifications
>> from working when the attribute agress with the mount option; otherwise,
>> they would be blocked by a btrfs.compression string that doesn't specify
>> a level.  That's a _regression_.
>
> Let me be this clear, the current one nor option 2 is not working either.
>
> If the current algo is different from the XATTR algo, it will be 
> whatever random number clamped to the XATTR algo for the current code.
>
> This applies to the option 2 solution. When mount option changed, the 
> level will suddenly change from whatever previous mount option to the 
> default.

TBF, I can see how it could be useful (or rather, how it could be good
to have it as an option) to have some files with btrfs.compression="zstd"
and then use -o compress= to decide on the fly how much compressed the
new data added to them should be.
Both are (read: will be, after the per-inode patch) 1 command away, but
there *might* be use-cases where mount is more suitable.


>>      https://wiki.tnonline.net/w/Btrfs/Compression
>
> The first URL doesn't even resolve here.
(for some reason it's down right now :(, it was up < 1 hour ago).

Best,
koraynilay

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

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

[Qu Wenruo]

在 2026/8/10 10:35, koraynilay 写道:
> On Mon Aug 10, 2026 at 2:51 AM CEST, Qu Wenruo wrote:
>>>>>> So either option 2 or 3 would be fine to me. Although I personally prefer
>>>>>> option 3 a little more, just because it's much cleaner code wise.
>>>>>
>>>>> Option 2 preserves legacy behavior that is 12 years old now, and it
>>>>> costs a single comparison in two 'if' statements.
>>>>>
>>>>> Option 3 makes an already confusing situation worse--it makes the
>>>>> underspecified behavior change depending on kernel version.
>>>>
>>>> One should never rely on something not documented in the first place.
>>>
>>> Option 3 prevents existing mount-option compression level specifications
>>> from working when the attribute agress with the mount option; otherwise,
>>> they would be blocked by a btrfs.compression string that doesn't specify
>>> a level.  That's a _regression_.
>>
>> Let me be this clear, the current one nor option 2 is not working either.
>>
>> If the current algo is different from the XATTR algo, it will be
>> whatever random number clamped to the XATTR algo for the current code.
>>
>> This applies to the option 2 solution. When mount option changed, the
>> level will suddenly change from whatever previous mount option to the
>> default.
> 
> TBF, I can see how it could be useful (or rather, how it could be good
> to have it as an option) to have some files with btrfs.compression="zstd"
> and then use -o compress= to decide on the fly how much compressed the
> new data added to them should be.
> Both are (read: will be, after the per-inode patch) 1 command away, but
> there *might* be use-cases where mount is more suitable.

To be honest, with the proper XATTR compression level specification, I 
think we should even deprecate compress= mount option, and make the 
XATTR one the only recommended way to specific compression.

There are already too many corner cases with mount option.

IMHO, a good design should allow and only allow the best way to do a thing.

And option 3 matches perfect for the XATTR only compression future. It 
still allows old XATTR to work, have a very sane default level, very 
explicit and clear independent from whatever stupid mount option there 
could be.

> 
> 
>>>       https://wiki.tnonline.net/w/Btrfs/Compression
>>
>> The first URL doesn't even resolve here.
> (for some reason it's down right now :(, it was up < 1 hour ago).
> 
> Best,
> koraynilay

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

[koraynilay]

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

On Mon Aug 10, 2026 at 3:54 AM CEST, Qu Wenruo wrote:
>
>
> 在 2026/8/10 10:35, koraynilay 写道:
>> On Mon Aug 10, 2026 at 2:51 AM CEST, Qu Wenruo wrote:
>>>>>>> So either option 2 or 3 would be fine to me. Although I personally prefer
>>>>>>> option 3 a little more, just because it's much cleaner code wise.
>>>>>>
>>>>>> Option 2 preserves legacy behavior that is 12 years old now, and it
>>>>>> costs a single comparison in two 'if' statements.
>>>>>>
>>>>>> Option 3 makes an already confusing situation worse--it makes the
>>>>>> underspecified behavior change depending on kernel version.
>>>>>
>>>>> One should never rely on something not documented in the first place.
>>>>
>>>> Option 3 prevents existing mount-option compression level specifications
>>>> from working when the attribute agress with the mount option; otherwise,
>>>> they would be blocked by a btrfs.compression string that doesn't specify
>>>> a level.  That's a _regression_.
>>>
>>> Let me be this clear, the current one nor option 2 is not working either.
>>>
>>> If the current algo is different from the XATTR algo, it will be
>>> whatever random number clamped to the XATTR algo for the current code.
>>>
>>> This applies to the option 2 solution. When mount option changed, the
>>> level will suddenly change from whatever previous mount option to the
>>> default.
>> 
>> TBF, I can see how it could be useful (or rather, how it could be good
>> to have it as an option) to have some files with btrfs.compression="zstd"
>> and then use -o compress= to decide on the fly how much compressed the
>> new data added to them should be.
>> Both are (read: will be, after the per-inode patch) 1 command away, but
>> there *might* be use-cases where mount is more suitable.
>
> To be honest, with the proper XATTR compression level specification, I 
> think we should even deprecate compress= mount option, and make the 
> XATTR one the only recommended way to specific compression.

Ah, and in that case, to set compression on the whole fs use btrfs prop to
set it on the root?

> There are already too many corner cases with mount option.
>
> IMHO, a good design should allow and only allow the best way to do a thing.
>
> And option 3 matches perfect for the XATTR only compression future. It 
> still allows old XATTR to work, have a very sane default level, very 
> explicit and clear independent from whatever stupid mount option there 
> could be.

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

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

[Qu Wenruo]

在 2026/8/10 11:27, koraynilay 写道:
> On Mon Aug 10, 2026 at 3:54 AM CEST, Qu Wenruo wrote:
>>
>>
>> 在 2026/8/10 10:35, koraynilay 写道:
>>> On Mon Aug 10, 2026 at 2:51 AM CEST, Qu Wenruo wrote:
>>>>>>>> So either option 2 or 3 would be fine to me. Although I personally prefer
>>>>>>>> option 3 a little more, just because it's much cleaner code wise.
>>>>>>>
>>>>>>> Option 2 preserves legacy behavior that is 12 years old now, and it
>>>>>>> costs a single comparison in two 'if' statements.
>>>>>>>
>>>>>>> Option 3 makes an already confusing situation worse--it makes the
>>>>>>> underspecified behavior change depending on kernel version.
>>>>>>
>>>>>> One should never rely on something not documented in the first place.
>>>>>
>>>>> Option 3 prevents existing mount-option compression level specifications
>>>>> from working when the attribute agress with the mount option; otherwise,
>>>>> they would be blocked by a btrfs.compression string that doesn't specify
>>>>> a level.  That's a _regression_.
>>>>
>>>> Let me be this clear, the current one nor option 2 is not working either.
>>>>
>>>> If the current algo is different from the XATTR algo, it will be
>>>> whatever random number clamped to the XATTR algo for the current code.
>>>>
>>>> This applies to the option 2 solution. When mount option changed, the
>>>> level will suddenly change from whatever previous mount option to the
>>>> default.
>>>
>>> TBF, I can see how it could be useful (or rather, how it could be good
>>> to have it as an option) to have some files with btrfs.compression="zstd"
>>> and then use -o compress= to decide on the fly how much compressed the
>>> new data added to them should be.
>>> Both are (read: will be, after the per-inode patch) 1 command away, but
>>> there *might* be use-cases where mount is more suitable.
>>
>> To be honest, with the proper XATTR compression level specification, I
>> think we should even deprecate compress= mount option, and make the
>> XATTR one the only recommended way to specific compression.
> 
> Ah, and in that case, to set compression on the whole fs use btrfs prop to
> set it on the root?

Yep.
> 
>> There are already too many corner cases with mount option.
>>
>> IMHO, a good design should allow and only allow the best way to do a thing.
>>
>> And option 3 matches perfect for the XATTR only compression future. It
>> still allows old XATTR to work, have a very sane default level, very
>> explicit and clear independent from whatever stupid mount option there
>> could be.

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

[Zygo Blaxell]

On Mon, Aug 10, 2026 at 11:27:49AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/8/10 11:27, koraynilay 写道:
> > On Mon Aug 10, 2026 at 3:54 AM CEST, Qu Wenruo wrote:
> > > 
> > > 
> > > 在 2026/8/10 10:35, koraynilay 写道:
> > > > On Mon Aug 10, 2026 at 2:51 AM CEST, Qu Wenruo wrote:
> > > > > > > > > So either option 2 or 3 would be fine to me. Although I personally prefer
> > > > > > > > > option 3 a little more, just because it's much cleaner code wise.
> > > > > > > > 
> > > > > > > > Option 2 preserves legacy behavior that is 12 years old now, and it
> > > > > > > > costs a single comparison in two 'if' statements.
> > > > > > > > 
> > > > > > > > Option 3 makes an already confusing situation worse--it makes the
> > > > > > > > underspecified behavior change depending on kernel version.
> > > > > > > 
> > > > > > > One should never rely on something not documented in the first place.
> > > > > > 
> > > > > > Option 3 prevents existing mount-option compression level specifications
> > > > > > from working when the attribute agress with the mount option; otherwise,
> > > > > > they would be blocked by a btrfs.compression string that doesn't specify
> > > > > > a level.  That's a _regression_.
> > > > > 
> > > > > Let me be this clear, the current one nor option 2 is not working either.
> > > > > 
> > > > > If the current algo is different from the XATTR algo, it will be
> > > > > whatever random number clamped to the XATTR algo for the current code.
> > > > > 
> > > > > This applies to the option 2 solution. When mount option changed, the
> > > > > level will suddenly change from whatever previous mount option to the
> > > > > default.
> > > > 
> > > > TBF, I can see how it could be useful (or rather, how it could be good
> > > > to have it as an option) to have some files with btrfs.compression="zstd"
> > > > and then use -o compress= to decide on the fly how much compressed the
> > > > new data added to them should be.
> > > > Both are (read: will be, after the per-inode patch) 1 command away, but
> > > > there *might* be use-cases where mount is more suitable.
> > > 
> > > To be honest, with the proper XATTR compression level specification, I
> > > think we should even deprecate compress= mount option, and make the
> > > XATTR one the only recommended way to specific compression.
> > 
> > Ah, and in that case, to set compression on the whole fs use btrfs prop to
> > set it on the root?
> 
> Yep.

I am vehemently opposed to deprecation of a feature that will require
updating _billions_ of inodes per server to get the same effect, when
the filesystem was previously able to handle a 4-level hierarchy of
compression options with "defer to next level" since the beginning.

I will maintain a fork if I have to.  Hard NAK.

We can have clearer documentation about how options are processed,
and clearly what options mean "look up to the next level" vs "use the
default" or "use the locally defined value."

> > > There are already too many corner cases with mount option.
> > > 
> > > IMHO, a good design should allow and only allow the best way to do a thing.
> > > 
> > > And option 3 matches perfect for the XATTR only compression future. It
> > > still allows old XATTR to work, have a very sane default level, very
> > > explicit and clear independent from whatever stupid mount option there
> > > could be.
> 
> 
> 

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

[Qu Wenruo]

在 2026/8/10 11:36, Zygo Blaxell 写道:
> On Mon, Aug 10, 2026 at 11:27:49AM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2026/8/10 11:27, koraynilay 写道:
>>> On Mon Aug 10, 2026 at 3:54 AM CEST, Qu Wenruo wrote:
>>>>
>>>>
>>>> 在 2026/8/10 10:35, koraynilay 写道:
>>>>> On Mon Aug 10, 2026 at 2:51 AM CEST, Qu Wenruo wrote:
>>>>>>>>>> So either option 2 or 3 would be fine to me. Although I personally prefer
>>>>>>>>>> option 3 a little more, just because it's much cleaner code wise.
>>>>>>>>>
>>>>>>>>> Option 2 preserves legacy behavior that is 12 years old now, and it
>>>>>>>>> costs a single comparison in two 'if' statements.
>>>>>>>>>
>>>>>>>>> Option 3 makes an already confusing situation worse--it makes the
>>>>>>>>> underspecified behavior change depending on kernel version.
>>>>>>>>
>>>>>>>> One should never rely on something not documented in the first place.
>>>>>>>
>>>>>>> Option 3 prevents existing mount-option compression level specifications
>>>>>>> from working when the attribute agress with the mount option; otherwise,
>>>>>>> they would be blocked by a btrfs.compression string that doesn't specify
>>>>>>> a level.  That's a _regression_.
>>>>>>
>>>>>> Let me be this clear, the current one nor option 2 is not working either.
>>>>>>
>>>>>> If the current algo is different from the XATTR algo, it will be
>>>>>> whatever random number clamped to the XATTR algo for the current code.
>>>>>>
>>>>>> This applies to the option 2 solution. When mount option changed, the
>>>>>> level will suddenly change from whatever previous mount option to the
>>>>>> default.
>>>>>
>>>>> TBF, I can see how it could be useful (or rather, how it could be good
>>>>> to have it as an option) to have some files with btrfs.compression="zstd"
>>>>> and then use -o compress= to decide on the fly how much compressed the
>>>>> new data added to them should be.
>>>>> Both are (read: will be, after the per-inode patch) 1 command away, but
>>>>> there *might* be use-cases where mount is more suitable.
>>>>
>>>> To be honest, with the proper XATTR compression level specification, I
>>>> think we should even deprecate compress= mount option, and make the
>>>> XATTR one the only recommended way to specific compression.
>>>
>>> Ah, and in that case, to set compression on the whole fs use btrfs prop to
>>> set it on the root?
>>
>> Yep.
> 
> I am vehemently opposed to deprecation of a feature that will require
> updating _billions_ of inodes per server to get the same effect, when
> the filesystem was previously able to handle a 4-level hierarchy of
> compression options with "defer to next level" since the beginning.
> 
> I will maintain a fork if I have to.  Hard NAK.

Do whatever you want.

> 
> We can have clearer documentation about how options are processed,
> and clearly what options mean "look up to the next level" vs "use the
> default" or "use the locally defined value."
> 
>>>> There are already too many corner cases with mount option.
>>>>
>>>> IMHO, a good design should allow and only allow the best way to do a thing.
>>>>
>>>> And option 3 matches perfect for the XATTR only compression future. It
>>>> still allows old XATTR to work, have a very sane default level, very
>>>> explicit and clear independent from whatever stupid mount option there
>>>> could be.
>>
>>
>>
> 

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

[koraynilay]

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

On Mon Aug 10, 2026 at 3:05 AM CEST, koraynilay wrote:
> On Mon Aug 10, 2026 at 2:51 AM CEST, Qu Wenruo wrote:
>>>>>> So either option 2 or 3 would be fine to me. Although I personally prefer
>>>>>> option 3 a little more, just because it's much cleaner code wise.
>>>>>
>>>>> Option 2 preserves legacy behavior that is 12 years old now, and it
>>>>> costs a single comparison in two 'if' statements.
>>>>>
>>>>> Option 3 makes an already confusing situation worse--it makes the
>>>>> underspecified behavior change depending on kernel version.
>>>>
>>>> One should never rely on something not documented in the first place.
>>> 
>>> Option 3 prevents existing mount-option compression level specifications
>>> from working when the attribute agress with the mount option; otherwise,
>>> they would be blocked by a btrfs.compression string that doesn't specify
>>> a level.  That's a _regression_.
>>
>> Let me be this clear, the current one nor option 2 is not working either.
>>
>> If the current algo is different from the XATTR algo, it will be 
>> whatever random number clamped to the XATTR algo for the current code.
>>
>> This applies to the option 2 solution. When mount option changed, the 
>> level will suddenly change from whatever previous mount option to the 
>> default.
>
> TBF, I can see how it could be useful (or rather, how it could be good
> to have it as an option) to have some files with btrfs.compression="zstd"
> and then use -o compress= to decide on the fly how much compressed the
> new data added to them should be.
> Both are (read: will be, after the per-inode patch) 1 command away, but
> there *might* be use-cases where mount is more suitable.
>
>

Hi,

after a little bit of discussion on #btrfs on IRC, kepstin and I drafted
a doc change[1] on a variant of option 2, a bit more clear and expanding
also on the per-inode level change.

Thanks


Best,
koraynilay

[1]: https://github.com/kdave/btrfs-progs/pull/1152

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