* [PATCH] btrfs: Change how child btrfs_inode inherits xattr compression from parent after expanding btrfs_info member prop_compress
@ 2022-01-23 12:43 Li Zhang
0 siblings, 0 replies; only message in thread
From: Li Zhang @ 2022-01-23 12:43 UTC (permalink / raw)
To: linux-btrfs; +Cc: zhanglikernel
Hi, the patch I submitted fails xfstest/btrfs/048.
[patch link]
https://lore.kernel.org/linux-btrfs/1642323009-1953-1-git-send-email-zhanglikernel@gmail.com/
This is due to the following reasons.
[Problem Description]
1. Expanded struct btrfs_info member prop_compress to record
compression type and level, but did not change prop_compress allocation and initialization.
2. Inheriting inode still considers the parent inode member prop_compress just
records the compression type, it is wrong to parse prop_compress to compressed xattr.
[Repair method]
1. If the struct btrfs_inode member prop_compress is assigned or initialized to
BTRFS_COMPRESS_NONE , it will combine the compression type and compression level 0.
2. If a btrfs_inode inherits properties from parent, resolve prop_compress
to xattr compression <compression_type>:<compression_type>
But this changes the way the child btrfs_inode inherits properties from the
parent node, because parsing prop_compress as xattr compression <compression_type>:<compression_type>
formats xattr compression and therefore changes the behavior of btrfs.
Specifically:
Old behavior:
$ mkdir <parent directory>
$ btrfs attribute set <parent_dir> compressed zstd
$ btrfs attribute gets <parent_dir>
compressed=zstd
$ mkdir <parent directory>/1
$ touch <parent directory>/2
$ btrfs property gets <parent_dir>/1
compressed=zstd
$ btrfs attribute gets <parent_dir>/2
compressed=zstd
New behavior:
$ mkdir <parent directory>
$ btrfs attribute set <parent_dir> compressed zstd
$ btrfs attribute gets <parent_dir>
compressed=zstd
$ mkdir <parent directory>/1
$ touch <parent directory>/2
$ btrfs property gets <parent_dir>/1
compressed=zstd:3
$ btrfs attribute gets <parent_dir>/2
compressed=zstd:3
[Puzzled]
1. Because it will format the child's btrfs_inode xattr compression,
it still fails the fstest/btrfs/048 test, what can I do to fix it completely?
2. The original way of parsing the struct btrfs_inode member prop_compress was
to get only the compression, so it could return a global compression description,
but now it will combine the compression type and compression level, so this patch returns a
dynamic memory buf and is used by the callee, which Is there a better way to return the compressed
description xattr string?
Reported-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
---
fs/btrfs/inode.c | 2 +-
fs/btrfs/props.c | 39 ++++++++++++++++++++++++++++++---------
2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index fb44899..16d40bd 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8796,7 +8796,7 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
btrfs_init_metadata_block_rsv(fs_info, &ei->block_rsv,
BTRFS_BLOCK_RSV_DELALLOC);
ei->runtime_flags = 0;
- ei->prop_compress = BTRFS_COMPRESS_NONE;
+ ei->prop_compress = btrfs_compress_combine_type_level(BTRFS_COMPRESS_NONE, 0);
ei->defrag_compress = BTRFS_COMPRESS_NONE;
ei->delayed_node = NULL;
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index f07be37..b6d8e6a 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -279,7 +279,7 @@ static int prop_compression_apply(struct inode *inode, const char *value,
if (len == 0) {
BTRFS_I(inode)->flags &= ~BTRFS_INODE_COMPRESS;
BTRFS_I(inode)->flags &= ~BTRFS_INODE_NOCOMPRESS;
- BTRFS_I(inode)->prop_compress = BTRFS_COMPRESS_NONE;
+ BTRFS_I(inode)->prop_compress = btrfs_compress_combine_type_level(BTRFS_COMPRESS_NONE, 0);
ret = 0;
goto out;
}
@@ -289,7 +289,7 @@ static int prop_compression_apply(struct inode *inode, const char *value,
(len == 4 && strncmp("none", value, 4) == 0)) {
BTRFS_I(inode)->flags |= BTRFS_INODE_NOCOMPRESS;
BTRFS_I(inode)->flags &= ~BTRFS_INODE_COMPRESS;
- BTRFS_I(inode)->prop_compress = BTRFS_COMPRESS_NONE;
+ BTRFS_I(inode)->prop_compress = btrfs_compress_combine_type_level(BTRFS_COMPRESS_NONE, 0);
ret = 0;
goto out;
}
@@ -332,15 +332,24 @@ static int prop_compression_apply(struct inode *inode, const char *value,
static const char *prop_compression_extract(struct inode *inode)
{
- switch (BTRFS_I(inode)->prop_compress) {
+ char *type_level = NULL;
+ int type_level_buf_len = 50;
+
+ type_level = (char *)kmalloc(type_level_buf_len, GFP_NOFS);
+ if (!type_level) {
+ return NULL;
+ }
+ switch (btrfs_compress_type(BTRFS_I(inode)->prop_compress)) {
case BTRFS_COMPRESS_ZLIB:
case BTRFS_COMPRESS_LZO:
case BTRFS_COMPRESS_ZSTD:
- return btrfs_compress_type2str(BTRFS_I(inode)->prop_compress);
+ return btrfs_compress_type_level2str(btrfs_compress_type(BTRFS_I(inode)->prop_compress), btrfs_compress_level(BTRFS_I(inode)->prop_compress), type_level, type_level_buf_len);
default:
break;
}
-
+ if (type_level) {
+ kfree(type_level);
+ }
return NULL;
}
@@ -371,22 +380,26 @@ static int inherit_props(struct btrfs_trans_handle *trans,
for (i = 0; i < ARRAY_SIZE(prop_handlers); i++) {
const struct prop_handler *h = &prop_handlers[i];
const char *value;
+ int is_compression_xattr = !strncmp(h->xattr_name, XATTR_BTRFS_PREFIX "compression", strlen(XATTR_BTRFS_PREFIX "compression"));
u64 num_bytes = 0;
if (!h->inheritable)
continue;
value = h->extract(parent);
+ btrfs_info(fs_info, "inherit_props values:%s", value);
if (!value)
continue;
-
/*
* This is not strictly necessary as the property should be
* valid, but in case it isn't, don't propagate it further.
*/
ret = h->validate(value, strlen(value));
- if (ret)
+ if (ret) {
+ if (is_compression_xattr)
+ kfree(value);
continue;
+ }
/*
* Currently callers should be reserving 1 item for properties,
@@ -400,8 +413,11 @@ static int inherit_props(struct btrfs_trans_handle *trans,
ret = btrfs_block_rsv_add(fs_info, trans->block_rsv,
num_bytes,
BTRFS_RESERVE_NO_FLUSH);
- if (ret)
+ if (ret) {
+ if (is_compression_xattr)
+ kfree(value);
return ret;
+ }
}
ret = btrfs_setxattr(trans, inode, h->xattr_name, value,
@@ -419,10 +435,15 @@ static int inherit_props(struct btrfs_trans_handle *trans,
if (need_reserve) {
btrfs_block_rsv_release(fs_info, trans->block_rsv,
num_bytes, NULL);
- if (ret)
+ if (ret) {
+ if (is_compression_xattr)
+ kfree(value);
return ret;
+ }
}
need_reserve = true;
+ if (is_compression_xattr)
+ kfree(value);
}
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2022-01-23 12:43 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-23 12:43 [PATCH] btrfs: Change how child btrfs_inode inherits xattr compression from parent after expanding btrfs_info member prop_compress Li Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).