2023-04-07 14:20:53

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH 0/7] erofs: introduce long xattr name prefixes feature

Background
=========
overlayfs uses xattrs to keep its own metadata. If such xattrs are
heavily used, such as Composefs model [1], large amount of xattrs
with diverse xattr values exist but only a few common xattr names
are valid (trusted.overlay.redirect, trusted.overlay.digest, and
maybe more in the future) . For example:

# file 1
trusted.overlay.redirect="xxx"

# file 2
trusted.overlay.redirect="yyy"

That makes the existing predefined prefixes ineffective in both image
size and runtime performance.

Solution Proposed
====================
Let's introduce long xattr name prefixes now to fix this. They work
similarly as the predefined name prefixes, except that long xattr name
prefixes are user specified.

When a long xattr name prefix is used, the shared long xattr prefixes
are stored in the packed or meta inode, while the remained trailing part
of the xattr name apart from the long xattr name prefix will be stored
in erofs_xattr_entry.e_name. e_name is empty if the xattr name matches
exactly as the long xattr name prefix.

Erofs image sizes will be smaller in the above described scenario where
all files have diverse xattr values with the same set of xattr names[2],
such as having following xattrs like:

trusted.overlay.metacopy=""
trusted.overlay.redirect="xxx"

Here are the image sizes for comparison (32-byte inodes are used):

```
7.4M large.erofs.T0.xattr
6.4M large.erofs.T0.xattr.share
```

- share: "--xattr-prefix=trusted.overlay.redirect" option of mkfs.erofs.
w/ this option, the long xattr name prefixes feature is enabled.

It can be seen ~14% disk space is saved with this feature in this
typical workload, therefore metadata I/Os could also be more effective
then.

Test
====
It passes erofs/019 of erofs-utils.


[1] https://lore.kernel.org/all/CAOQ4uxgGc33_QVBXMbQTnmbpHio4amv=W7ax2vQ1UMet0k_KoA@mail.gmail.com/
[2] https://my.owndrive.com/index.php/s/irHJXRpZHtT3a5i



Gao Xiang (1):
erofs: keep meta inode into erofs_buf

Jingbo Xu (6):
erofs: initialize packed inode after root inode is assigned
erofs: move packed inode out of the compression part
erofs: introduce on-disk format for long xattr name prefixes
erofs: add helpers to load long xattr name prefixes
erofs: handle long xattr name prefixes properly
erofs: enable long extended attribute name prefixes

fs/erofs/data.c | 23 +++++----
fs/erofs/dir.c | 3 +-
fs/erofs/erofs_fs.h | 20 +++++++-
fs/erofs/internal.h | 20 ++++++--
fs/erofs/namei.c | 4 +-
fs/erofs/super.c | 42 +++++++++-------
fs/erofs/xattr.c | 116 +++++++++++++++++++++++++++++++++++++++++---
fs/erofs/xattr.h | 4 ++
fs/erofs/zdata.c | 4 +-
9 files changed, 192 insertions(+), 44 deletions(-)

--
2.19.1.6.gb485710b


2023-04-07 14:21:04

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH 3/7] erofs: move packed inode out of the compression part

packed inode could be used in more scenarios which are independent of
compression in the future.

For example, packed inode could be used to keep extra long xattr
prefixes with the help of following patches.

Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/internal.h | 2 +-
fs/erofs/super.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index caea9dc1cd82..8b5168f94dd2 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -134,8 +134,8 @@ struct erofs_sb_info {
struct inode *managed_cache;

struct erofs_sb_lz4_info lz4;
- struct inode *packed_inode;
#endif /* CONFIG_EROFS_FS_ZIP */
+ struct inode *packed_inode;
struct erofs_dev_context *devs;
struct dax_device *dax_dev;
u64 dax_part_off;
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 325602820dc8..8f2f8433db61 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -810,7 +810,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)

erofs_shrinker_register(sb);
/* sb->s_umount is already locked, SB_ACTIVE and SB_BORN are not set */
-#ifdef CONFIG_EROFS_FS_ZIP
if (erofs_sb_has_fragments(sbi) && sbi->packed_nid) {
sbi->packed_inode = erofs_iget(sb, sbi->packed_nid);
if (IS_ERR(sbi->packed_inode)) {
@@ -819,7 +818,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
return err;
}
}
-#endif
err = erofs_init_managed_cache(sb);
if (err)
return err;
@@ -986,9 +984,9 @@ static void erofs_put_super(struct super_block *sb)
#ifdef CONFIG_EROFS_FS_ZIP
iput(sbi->managed_cache);
sbi->managed_cache = NULL;
+#endif
iput(sbi->packed_inode);
sbi->packed_inode = NULL;
-#endif
erofs_free_dev_context(sbi->devs);
sbi->devs = NULL;
erofs_fscache_unregister_fs(sb);
--
2.19.1.6.gb485710b

2023-04-07 14:21:05

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH 1/7] erofs: keep meta inode into erofs_buf

From: Gao Xiang <[email protected]>

So that erofs_read_metadata() can read metadata from other inodes
(e.g. packed inode) as well.

Signed-off-by: Gao Xiang <[email protected]>
---
fs/erofs/data.c | 23 ++++++++++++++---------
fs/erofs/dir.c | 3 ++-
fs/erofs/internal.h | 6 ++++--
fs/erofs/namei.c | 4 +++-
fs/erofs/super.c | 6 +++---
fs/erofs/zdata.c | 4 ++--
6 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index e5458b4c3d0c..aa7f9e4f86fb 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -31,11 +31,11 @@ void erofs_put_metabuf(struct erofs_buf *buf)
* Derive the block size from inode->i_blkbits to make compatible with
* anonymous inode in fscache mode.
*/
-void *erofs_bread(struct erofs_buf *buf, struct inode *inode,
- erofs_blk_t blkaddr, enum erofs_kmap_type type)
+void *erofs_bread(struct erofs_buf *buf, erofs_blk_t blkaddr,
+ enum erofs_kmap_type type)
{
+ struct inode *inode = buf->inode;
erofs_off_t offset = blkaddr << inode->i_blkbits;
- struct address_space *const mapping = inode->i_mapping;
pgoff_t index = offset >> PAGE_SHIFT;
struct page *page = buf->page;
struct folio *folio;
@@ -45,7 +45,7 @@ void *erofs_bread(struct erofs_buf *buf, struct inode *inode,
erofs_put_metabuf(buf);

nofs_flag = memalloc_nofs_save();
- folio = read_cache_folio(mapping, index, NULL, NULL);
+ folio = read_cache_folio(inode->i_mapping, index, NULL, NULL);
memalloc_nofs_restore(nofs_flag);
if (IS_ERR(folio))
return folio;
@@ -67,14 +67,19 @@ void *erofs_bread(struct erofs_buf *buf, struct inode *inode,
return buf->base + (offset & ~PAGE_MASK);
}

-void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
- erofs_blk_t blkaddr, enum erofs_kmap_type type)
+void erofs_init_metabuf(struct erofs_buf *buf, struct super_block *sb)
{
if (erofs_is_fscache_mode(sb))
- return erofs_bread(buf, EROFS_SB(sb)->s_fscache->inode,
- blkaddr, type);
+ buf->inode = EROFS_SB(sb)->s_fscache->inode;
+ else
+ buf->inode = sb->s_bdev->bd_inode;
+}

- return erofs_bread(buf, sb->s_bdev->bd_inode, blkaddr, type);
+void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
+ erofs_blk_t blkaddr, enum erofs_kmap_type type)
+{
+ erofs_init_metabuf(buf, sb);
+ return erofs_bread(buf, blkaddr, type);
}

static int erofs_map_blocks_flatmode(struct inode *inode,
diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index 963bbed0b699..b80abec0531a 100644
--- a/fs/erofs/dir.c
+++ b/fs/erofs/dir.c
@@ -58,11 +58,12 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
int err = 0;
bool initial = true;

+ buf.inode = dir;
while (ctx->pos < dirsize) {
struct erofs_dirent *de;
unsigned int nameoff, maxsize;

- de = erofs_bread(&buf, dir, i, EROFS_KMAP);
+ de = erofs_bread(&buf, i, EROFS_KMAP);
if (IS_ERR(de)) {
erofs_err(sb, "fail to readdir of logical block %u of nid %llu",
i, EROFS_I(dir)->nid);
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index e30a4fd43ccb..2bcff3194e4a 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -247,6 +247,7 @@ enum erofs_kmap_type {
};

struct erofs_buf {
+ struct inode *inode;
struct page *page;
void *base;
enum erofs_kmap_type kmap_type;
@@ -440,8 +441,9 @@ extern const struct iomap_ops z_erofs_iomap_report_ops;

void erofs_unmap_metabuf(struct erofs_buf *buf);
void erofs_put_metabuf(struct erofs_buf *buf);
-void *erofs_bread(struct erofs_buf *buf, struct inode *inode,
- erofs_blk_t blkaddr, enum erofs_kmap_type type);
+void *erofs_bread(struct erofs_buf *buf, erofs_blk_t blkaddr,
+ enum erofs_kmap_type type);
+void erofs_init_metabuf(struct erofs_buf *buf, struct super_block *sb);
void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
erofs_blk_t blkaddr, enum erofs_kmap_type type);
int erofs_map_dev(struct super_block *sb, struct erofs_map_dev *dev);
diff --git a/fs/erofs/namei.c b/fs/erofs/namei.c
index f091e9a0f0a1..43096bac4c99 100644
--- a/fs/erofs/namei.c
+++ b/fs/erofs/namei.c
@@ -99,7 +99,8 @@ static void *erofs_find_target_block(struct erofs_buf *target,
struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
struct erofs_dirent *de;

- de = erofs_bread(&buf, dir, mid, EROFS_KMAP);
+ buf.inode = dir;
+ de = erofs_bread(&buf, mid, EROFS_KMAP);
if (!IS_ERR(de)) {
const int nameoff = nameoff_from_disk(de->nameoff, bsz);
const int ndirents = nameoff / sizeof(*de);
@@ -170,6 +171,7 @@ int erofs_namei(struct inode *dir, const struct qstr *name, erofs_nid_t *nid,

qn.name = name->name;
qn.end = name->name + name->len;
+ buf.inode = dir;

ndirents = 0;
de = erofs_find_target_block(&buf, dir, &qn, &ndirents);
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 9e56a6fb2267..58ffbf410bfb 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -135,7 +135,7 @@ static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
int len, i, cnt;

*offset = round_up(*offset, 4);
- ptr = erofs_read_metabuf(buf, sb, erofs_blknr(sb, *offset), EROFS_KMAP);
+ ptr = erofs_bread(buf, erofs_blknr(sb, *offset), EROFS_KMAP);
if (IS_ERR(ptr))
return ptr;

@@ -151,8 +151,7 @@ static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
for (i = 0; i < len; i += cnt) {
cnt = min_t(int, sb->s_blocksize - erofs_blkoff(sb, *offset),
len - i);
- ptr = erofs_read_metabuf(buf, sb, erofs_blknr(sb, *offset),
- EROFS_KMAP);
+ ptr = erofs_bread(buf, erofs_blknr(sb, *offset), EROFS_KMAP);
if (IS_ERR(ptr)) {
kfree(buffer);
return ptr;
@@ -179,6 +178,7 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
return -EINVAL;
}

+ erofs_init_metabuf(&buf, sb);
offset = EROFS_SUPER_OFFSET + sbi->sb_size;
alg = 0;
for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index a90d37c7bdd7..34944e400037 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -939,12 +939,12 @@ static int z_erofs_read_fragment(struct inode *inode, erofs_off_t pos,
if (!packed_inode)
return -EFSCORRUPTED;

+ buf.inode = packed_inode;
pos += EROFS_I(inode)->z_fragmentoff;
for (i = 0; i < len; i += cnt) {
cnt = min_t(unsigned int, len - i,
sb->s_blocksize - erofs_blkoff(sb, pos));
- src = erofs_bread(&buf, packed_inode,
- erofs_blknr(sb, pos), EROFS_KMAP);
+ src = erofs_bread(&buf, erofs_blknr(sb, pos), EROFS_KMAP);
if (IS_ERR(src)) {
erofs_put_metabuf(&buf);
return PTR_ERR(src);
--
2.19.1.6.gb485710b

2023-04-07 14:21:10

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH 5/7] erofs: add helpers to load long xattr name prefixes

Long xattr name prefixes will be scanned upon mounting and the in-memory
long xattr name prefix array will be initialized accordingly.

Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/internal.h | 10 ++++++++
fs/erofs/super.c | 6 ++---
fs/erofs/xattr.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
fs/erofs/xattr.h | 4 ++++
4 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 8b5168f94dd2..5a9c19654b19 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -117,6 +117,11 @@ struct erofs_fscache {
char *name;
};

+struct erofs_xattr_prefix_item {
+ struct erofs_xattr_long_prefix *prefix;
+ u8 infix_len;
+};
+
struct erofs_sb_info {
struct erofs_mount_opts opt; /* options */
#ifdef CONFIG_EROFS_FS_ZIP
@@ -145,6 +150,9 @@ struct erofs_sb_info {
u32 meta_blkaddr;
#ifdef CONFIG_EROFS_FS_XATTR
u32 xattr_blkaddr;
+ u32 xattr_prefix_start;
+ u8 xattr_prefix_count;
+ struct erofs_xattr_prefix_item *xattr_prefixes;
#endif
u16 device_id_mask; /* valid bits of device id to be used */

@@ -440,6 +448,8 @@ extern const struct iomap_ops z_erofs_iomap_report_ops;
#define EROFS_REG_COOKIE_SHARE 0x0001
#define EROFS_REG_COOKIE_NEED_NOEXIST 0x0002

+void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
+ erofs_off_t *offset, int *lengthp);
void erofs_unmap_metabuf(struct erofs_buf *buf);
void erofs_put_metabuf(struct erofs_buf *buf);
void *erofs_bread(struct erofs_buf *buf, erofs_blk_t blkaddr,
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 8f2f8433db61..bf396e0c243a 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -126,10 +126,9 @@ static bool check_layout_compatibility(struct super_block *sb,
return true;
}

-#ifdef CONFIG_EROFS_FS_ZIP
/* read variable-sized metadata, offset will be aligned by 4-byte */
-static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
- erofs_off_t *offset, int *lengthp)
+void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
+ erofs_off_t *offset, int *lengthp)
{
u8 *buffer, *ptr;
int len, i, cnt;
@@ -162,6 +161,7 @@ static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
return buffer;
}

+#ifdef CONFIG_EROFS_FS_ZIP
static int erofs_load_compr_cfgs(struct super_block *sb,
struct erofs_super_block *dsb)
{
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index d76b74ece2e5..684571e83a2c 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -610,6 +610,62 @@ ssize_t erofs_listxattr(struct dentry *dentry,
return ret;
}

+void erofs_xattr_prefixes_cleanup(struct super_block *sb)
+{
+ struct erofs_sb_info *sbi = EROFS_SB(sb);
+ int i;
+
+ if (sbi->xattr_prefixes) {
+ for (i = 0; i < sbi->xattr_prefix_count; i++)
+ kfree(sbi->xattr_prefixes[i].prefix);
+ kfree(sbi->xattr_prefixes);
+ sbi->xattr_prefixes = NULL;
+ }
+}
+
+int erofs_xattr_prefixes_init(struct super_block *sb)
+{
+ struct erofs_sb_info *sbi = EROFS_SB(sb);
+ struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
+ erofs_off_t pos = (erofs_off_t)sbi->xattr_prefix_start << 2;
+ struct erofs_xattr_prefix_item *pfs;
+ int ret = 0, i, len;
+
+ if (!sbi->xattr_prefix_count)
+ return 0;
+
+ pfs = kzalloc(sbi->xattr_prefix_count * sizeof(*pfs), GFP_KERNEL);
+ if (!pfs)
+ return -ENOMEM;
+
+ if (erofs_sb_has_fragments(sbi))
+ buf.inode = sbi->packed_inode;
+ else
+ erofs_init_metabuf(&buf, sb);
+
+ for (i = 0; i < sbi->xattr_prefix_count; i++) {
+ void *ptr = erofs_read_metadata(sb, &buf, &pos, &len);
+
+ if (IS_ERR(ptr)) {
+ ret = PTR_ERR(ptr);
+ break;
+ } else if (len < sizeof(*pfs->prefix) ||
+ len > EROFS_NAME_LEN + sizeof(*pfs->prefix)) {
+ kfree(ptr);
+ ret = -EFSCORRUPTED;
+ break;
+ }
+ pfs[i].prefix = ptr;
+ pfs[i].infix_len = len - sizeof(struct erofs_xattr_long_prefix);
+ }
+
+ erofs_put_metabuf(&buf);
+ sbi->xattr_prefixes = pfs;
+ if (ret)
+ erofs_xattr_prefixes_cleanup(sb);
+ return ret;
+}
+
#ifdef CONFIG_EROFS_FS_POSIX_ACL
struct posix_acl *erofs_get_acl(struct inode *inode, int type, bool rcu)
{
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index a65158cba14f..e1265351aedd 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -40,9 +40,13 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)

extern const struct xattr_handler *erofs_xattr_handlers[];

+int erofs_xattr_prefixes_init(struct super_block *sb);
+void erofs_xattr_prefixes_cleanup(struct super_block *sb);
int erofs_getxattr(struct inode *, int, const char *, void *, size_t);
ssize_t erofs_listxattr(struct dentry *, char *, size_t);
#else
+static inline int erofs_xattr_prefixes_init(struct super_block *sb) { return 0; }
+static inline void erofs_xattr_prefixes_cleanup(struct super_block *sb) {}
static inline int erofs_getxattr(struct inode *inode, int index,
const char *name, void *buffer,
size_t buffer_size)
--
2.19.1.6.gb485710b

2023-04-07 14:21:11

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH 6/7] erofs: handle long xattr name prefixes properly

Make .{list,get}xattr routines adapted to long xattr name prefixes.
When the bit 7 of erofs_xattr_entry.e_name_index is set, it indicates
that it refers to a long xattr name prefix.

Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/xattr.c | 60 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 52 insertions(+), 8 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 684571e83a2c..8d81593655e8 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -301,11 +301,39 @@ struct getxattr_iter {
struct qstr name;
};

+static int erofs_xattr_long_entrymatch(struct getxattr_iter *it,
+ struct erofs_xattr_entry *entry)
+{
+ struct erofs_sb_info *sbi = EROFS_SB(it->it.sb);
+ u8 idx = entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK;
+ struct erofs_xattr_prefix_item *pf;
+
+ if (idx >= sbi->xattr_prefix_count)
+ return -ENOATTR;
+
+ pf = &sbi->xattr_prefixes[idx];
+ if (it->index != pf->prefix->base_index)
+ return -ENOATTR;
+
+ if (strncmp(it->name.name, pf->prefix->infix, pf->infix_len))
+ return -ENOATTR;
+
+ it->name.name += pf->infix_len;
+ it->name.len -= pf->infix_len;
+ if (it->name.len != entry->e_name_len)
+ return -ENOATTR;
+ return 0;
+}
+
static int xattr_entrymatch(struct xattr_iter *_it,
struct erofs_xattr_entry *entry)
{
struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);

+ /* should also match the infix for long name prefixes */
+ if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX)
+ return erofs_xattr_long_entrymatch(it, entry);
+
return (it->index != entry->e_name_index ||
it->name.len != entry->e_name_len) ? -ENOATTR : 0;
}
@@ -487,12 +515,26 @@ static int xattr_entrylist(struct xattr_iter *_it,
{
struct listxattr_iter *it =
container_of(_it, struct listxattr_iter, it);
- unsigned int prefix_len;
- const char *prefix;
-
- const struct xattr_handler *h =
- erofs_xattr_handler(entry->e_name_index);
+ unsigned int base_index = entry->e_name_index;
+ unsigned int prefix_len, infix_len = 0;
+ const char *prefix, *infix = NULL;
+ const struct xattr_handler *h;
+
+ if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) {
+ struct erofs_sb_info *sbi = EROFS_SB(_it->sb);
+ u8 idx = entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK;
+ struct erofs_xattr_prefix_item *pf;
+
+ if (idx >= sbi->xattr_prefix_count)
+ return 1;
+
+ pf = &sbi->xattr_prefixes[idx];
+ infix = pf->prefix->infix;
+ infix_len = pf->infix_len;
+ base_index = pf->prefix->base_index;
+ }

+ h = erofs_xattr_handler(base_index);
if (!h || (h->list && !h->list(it->dentry)))
return 1;

@@ -500,16 +542,18 @@ static int xattr_entrylist(struct xattr_iter *_it,
prefix_len = strlen(prefix);

if (!it->buffer) {
- it->buffer_ofs += prefix_len + entry->e_name_len + 1;
+ it->buffer_ofs += prefix_len + infix_len +
+ entry->e_name_len + 1;
return 1;
}

- if (it->buffer_ofs + prefix_len
+ if (it->buffer_ofs + prefix_len + infix_len +
+ entry->e_name_len + 1 > it->buffer_size)
return -ERANGE;

memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
- it->buffer_ofs += prefix_len;
+ memcpy(it->buffer + it->buffer_ofs + prefix_len, infix, infix_len);
+ it->buffer_ofs += prefix_len + infix_len;
return 0;
}

--
2.19.1.6.gb485710b

2023-04-07 14:21:49

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH 7/7] erofs: enable long extended attribute name prefixes

Let's enable long xattr name prefix feature. Old kernels will just
ignore / skip such extended attributes so that in case you don't want
to mount such images. Add another incompatible feature as an option
for this.

Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/erofs_fs.h | 4 +++-
fs/erofs/internal.h | 1 +
fs/erofs/super.c | 8 ++++++++
3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index ea62f83dac40..ac42a7255b39 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -27,6 +27,7 @@
#define EROFS_FEATURE_INCOMPAT_ZTAILPACKING 0x00000010
#define EROFS_FEATURE_INCOMPAT_FRAGMENTS 0x00000020
#define EROFS_FEATURE_INCOMPAT_DEDUPE 0x00000020
+#define EROFS_FEATURE_INCOMPAT_XATTR_PREFIXES 0x00000040
#define EROFS_ALL_FEATURE_INCOMPAT \
(EROFS_FEATURE_INCOMPAT_ZERO_PADDING | \
EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
@@ -36,7 +37,8 @@
EROFS_FEATURE_INCOMPAT_COMPR_HEAD2 | \
EROFS_FEATURE_INCOMPAT_ZTAILPACKING | \
EROFS_FEATURE_INCOMPAT_FRAGMENTS | \
- EROFS_FEATURE_INCOMPAT_DEDUPE)
+ EROFS_FEATURE_INCOMPAT_DEDUPE | \
+ EROFS_FEATURE_INCOMPAT_XATTR_PREFIXES)

#define EROFS_SB_EXTSLOT_SIZE 16

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 5a9c19654b19..f675050af2bb 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -285,6 +285,7 @@ EROFS_FEATURE_FUNCS(compr_head2, incompat, INCOMPAT_COMPR_HEAD2)
EROFS_FEATURE_FUNCS(ztailpacking, incompat, INCOMPAT_ZTAILPACKING)
EROFS_FEATURE_FUNCS(fragments, incompat, INCOMPAT_FRAGMENTS)
EROFS_FEATURE_FUNCS(dedupe, incompat, INCOMPAT_DEDUPE)
+EROFS_FEATURE_FUNCS(xattr_prefixes, incompat, INCOMPAT_XATTR_PREFIXES)
EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)

/* atomic flag definitions */
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index bf396e0c243a..8f85cc6162e2 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -391,6 +391,9 @@ static int erofs_read_superblock(struct super_block *sb)
sbi->packed_nid = le64_to_cpu(dsb->packed_nid);
sbi->inos = le64_to_cpu(dsb->inos);

+ sbi->xattr_prefix_start = le32_to_cpu(dsb->xattr_prefix_start);
+ sbi->xattr_prefix_count = dsb->xattr_prefix_count;
+
sbi->build_time = le64_to_cpu(dsb->build_time);
sbi->build_time_nsec = le32_to_cpu(dsb->build_time_nsec);

@@ -822,6 +825,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
if (err)
return err;

+ err = erofs_xattr_prefixes_init(sb);
+ if (err)
+ return err;
+
err = erofs_register_sysfs(sb);
if (err)
return err;
@@ -981,6 +988,7 @@ static void erofs_put_super(struct super_block *sb)

erofs_unregister_sysfs(sb);
erofs_shrinker_unregister(sb);
+ erofs_xattr_prefixes_cleanup(sb);
#ifdef CONFIG_EROFS_FS_ZIP
iput(sbi->managed_cache);
sbi->managed_cache = NULL;
--
2.19.1.6.gb485710b

2023-04-07 14:22:33

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes

Besides the predefined xattr name prefixes, introduces long xattr name
prefixes, which work similarly as the predefined name prefixes, except
that they are user specified.

It is especially useful for use cases together with overlayfs like
Composefs model, which introduces diverse xattr values with only a few
common xattr names (trusted.overlay.redirect, trusted.overlay.digest,
and maybe more in the future). That makes the existing predefined
prefixes ineffective in both image size and runtime performance.

When a user specified long xattr name prefix is used, only the trailing
part of the xattr name apart from the long xattr name prefix will be
stored in erofs_xattr_entry.e_name. e_name is empty if the xattr name
matches exactly as the long xattr name prefix. All long xattr prefixes
are stored in the packed or meta inode, which depends if fragments
feature is enabled or not.

For each long xattr name prefix, the on-disk format is kept as the same
as the unique metadata format: ALIGN({__le16 len, data}, 4), where len
represents the total size of struct erofs_xattr_long_prefix, followed
by data of struct erofs_xattr_long_prefix itself.

Each erofs_xattr_long_prefix keeps predefined prefixes (base_index)
and the remaining prefix string without the trailing '\0'.

Two fields are introduced to the on-disk superblock, where
xattr_prefix_count represents the total number of the long xattr name
prefixes recorded, and xattr_prefix_start represents the start offset of
recorded name prefixes in the packed/meta inode divided by 4.

When referring to a long xattr name prefix, the highest bit (bit 7) of
erofs_xattr_entry.e_name_index is set, while the lower bits (bit 0-6)
as a whole represents the index of the referred long name prefix among
all long xattr name prefixes.

Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/erofs_fs.h | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 44876a97cabd..ea62f83dac40 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -76,7 +76,8 @@ struct erofs_super_block {
__le16 extra_devices; /* # of devices besides the primary device */
__le16 devt_slotoff; /* startoff = devt_slotoff * devt_slotsize */
__u8 dirblkbits; /* directory block size in bit shift */
- __u8 reserved[5];
+ __u8 xattr_prefix_count; /* # of long xattr name prefixes */
+ __le32 xattr_prefix_start; /* start of long xattr prefixes */
__le64 packed_nid; /* nid of the special packed inode */
__u8 reserved2[24];
};
@@ -229,6 +230,13 @@ struct erofs_xattr_ibody_header {
#define EROFS_XATTR_INDEX_LUSTRE 5
#define EROFS_XATTR_INDEX_SECURITY 6

+/*
+ * bit 7 of e_name_index is set when it refers to a long xattr name prefix,
+ * while the remained lower bits represent the index of the prefix.
+ */
+#define EROFS_XATTR_LONG_PREFIX 0x80
+#define EROFS_XATTR_LONG_PREFIX_MASK 0x7f
+
/* xattr entry (for both inline & shared xattrs) */
struct erofs_xattr_entry {
__u8 e_name_len; /* length of name */
@@ -238,6 +246,12 @@ struct erofs_xattr_entry {
char e_name[]; /* attribute name */
};

+/* long xattr name prefix */
+struct erofs_xattr_long_prefix {
+ __u8 base_index; /* short xattr name prefix index */
+ char infix[]; /* infix apart from short prefix */
+};
+
static inline unsigned int erofs_xattr_ibody_size(__le16 i_xattr_icount)
{
if (!i_xattr_icount)
--
2.19.1.6.gb485710b

2023-04-07 17:37:11

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 7/7] erofs: enable long extended attribute name prefixes

Hi Jingbo,

kernel test robot noticed the following build errors:

[auto build test ERROR on xiang-erofs/dev-test]
[also build test ERROR on xiang-erofs/dev]
[cannot apply to xiang-erofs/fixes linus/master v6.3-rc5 next-20230406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Jingbo-Xu/erofs-keep-meta-inode-into-erofs_buf/20230407-221839
base: https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test
patch link: https://lore.kernel.org/r/20230407141710.113882-8-jefflexu%40linux.alibaba.com
patch subject: [PATCH 7/7] erofs: enable long extended attribute name prefixes
config: alpha-randconfig-r026-20230403 (https://download.01.org/0day-ci/archive/20230408/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/8cd5bbc6f857d54388099c30c3e3a48fdb15c283
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jingbo-Xu/erofs-keep-meta-inode-into-erofs_buf/20230407-221839
git checkout 8cd5bbc6f857d54388099c30c3e3a48fdb15c283
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash fs/erofs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

fs/erofs/super.c: In function 'erofs_read_superblock':
>> fs/erofs/super.c:394:12: error: 'struct erofs_sb_info' has no member named 'xattr_prefix_start'
394 | sbi->xattr_prefix_start = le32_to_cpu(dsb->xattr_prefix_start);
| ^~
>> fs/erofs/super.c:395:12: error: 'struct erofs_sb_info' has no member named 'xattr_prefix_count'
395 | sbi->xattr_prefix_count = dsb->xattr_prefix_count;
| ^~


vim +394 fs/erofs/super.c

333
334 static int erofs_read_superblock(struct super_block *sb)
335 {
336 struct erofs_sb_info *sbi;
337 struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
338 struct erofs_super_block *dsb;
339 void *data;
340 int ret;
341
342 data = erofs_read_metabuf(&buf, sb, 0, EROFS_KMAP);
343 if (IS_ERR(data)) {
344 erofs_err(sb, "cannot read erofs superblock");
345 return PTR_ERR(data);
346 }
347
348 sbi = EROFS_SB(sb);
349 dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);
350
351 ret = -EINVAL;
352 if (le32_to_cpu(dsb->magic) != EROFS_SUPER_MAGIC_V1) {
353 erofs_err(sb, "cannot find valid erofs superblock");
354 goto out;
355 }
356
357 sbi->blkszbits = dsb->blkszbits;
358 if (sbi->blkszbits < 9 || sbi->blkszbits > PAGE_SHIFT) {
359 erofs_err(sb, "blkszbits %u isn't supported", sbi->blkszbits);
360 goto out;
361 }
362 if (dsb->dirblkbits) {
363 erofs_err(sb, "dirblkbits %u isn't supported", dsb->dirblkbits);
364 goto out;
365 }
366
367 sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
368 if (erofs_sb_has_sb_chksum(sbi)) {
369 ret = erofs_superblock_csum_verify(sb, data);
370 if (ret)
371 goto out;
372 }
373
374 ret = -EINVAL;
375 if (!check_layout_compatibility(sb, dsb))
376 goto out;
377
378 sbi->sb_size = 128 + dsb->sb_extslots * EROFS_SB_EXTSLOT_SIZE;
379 if (sbi->sb_size > PAGE_SIZE - EROFS_SUPER_OFFSET) {
380 erofs_err(sb, "invalid sb_extslots %u (more than a fs block)",
381 sbi->sb_size);
382 goto out;
383 }
384 sbi->primarydevice_blocks = le32_to_cpu(dsb->blocks);
385 sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
386 #ifdef CONFIG_EROFS_FS_XATTR
387 sbi->xattr_blkaddr = le32_to_cpu(dsb->xattr_blkaddr);
388 #endif
389 sbi->islotbits = ilog2(sizeof(struct erofs_inode_compact));
390 sbi->root_nid = le16_to_cpu(dsb->root_nid);
391 sbi->packed_nid = le64_to_cpu(dsb->packed_nid);
392 sbi->inos = le64_to_cpu(dsb->inos);
393
> 394 sbi->xattr_prefix_start = le32_to_cpu(dsb->xattr_prefix_start);
> 395 sbi->xattr_prefix_count = dsb->xattr_prefix_count;
396
397 sbi->build_time = le64_to_cpu(dsb->build_time);
398 sbi->build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
399
400 memcpy(&sb->s_uuid, dsb->uuid, sizeof(dsb->uuid));
401
402 ret = strscpy(sbi->volume_name, dsb->volume_name,
403 sizeof(dsb->volume_name));
404 if (ret < 0) { /* -E2BIG */
405 erofs_err(sb, "bad volume name without NIL terminator");
406 ret = -EFSCORRUPTED;
407 goto out;
408 }
409
410 /* parse on-disk compression configurations */
411 if (erofs_sb_has_compr_cfgs(sbi))
412 ret = erofs_load_compr_cfgs(sb, dsb);
413 else
414 ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
415 if (ret < 0)
416 goto out;
417
418 /* handle multiple devices */
419 ret = erofs_scan_devices(sb, dsb);
420
421 if (erofs_sb_has_ztailpacking(sbi))
422 erofs_info(sb, "EXPERIMENTAL compressed inline data feature in use. Use at your own risk!");
423 if (erofs_is_fscache_mode(sb))
424 erofs_info(sb, "EXPERIMENTAL fscache-based on-demand read feature in use. Use at your own risk!");
425 if (erofs_sb_has_fragments(sbi))
426 erofs_info(sb, "EXPERIMENTAL compressed fragments feature in use. Use at your own risk!");
427 if (erofs_sb_has_dedupe(sbi))
428 erofs_info(sb, "EXPERIMENTAL global deduplication feature in use. Use at your own risk!");
429 out:
430 erofs_put_metabuf(&buf);
431 return ret;
432 }
433

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-04-07 18:39:09

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 7/7] erofs: enable long extended attribute name prefixes

Hi Jingbo,

kernel test robot noticed the following build errors:

[auto build test ERROR on xiang-erofs/dev-test]
[also build test ERROR on xiang-erofs/dev]
[cannot apply to xiang-erofs/fixes linus/master v6.3-rc5 next-20230406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Jingbo-Xu/erofs-keep-meta-inode-into-erofs_buf/20230407-221839
base: https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test
patch link: https://lore.kernel.org/r/20230407141710.113882-8-jefflexu%40linux.alibaba.com
patch subject: [PATCH 7/7] erofs: enable long extended attribute name prefixes
config: x86_64-randconfig-a005-20230403 (https://download.01.org/0day-ci/archive/20230408/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/8cd5bbc6f857d54388099c30c3e3a48fdb15c283
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jingbo-Xu/erofs-keep-meta-inode-into-erofs_buf/20230407-221839
git checkout 8cd5bbc6f857d54388099c30c3e3a48fdb15c283
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> fs/erofs/super.c:394:7: error: no member named 'xattr_prefix_start' in 'struct erofs_sb_info'
sbi->xattr_prefix_start = le32_to_cpu(dsb->xattr_prefix_start);
~~~ ^
>> fs/erofs/super.c:395:7: error: no member named 'xattr_prefix_count' in 'struct erofs_sb_info'
sbi->xattr_prefix_count = dsb->xattr_prefix_count;
~~~ ^
2 errors generated.


vim +394 fs/erofs/super.c

333
334 static int erofs_read_superblock(struct super_block *sb)
335 {
336 struct erofs_sb_info *sbi;
337 struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
338 struct erofs_super_block *dsb;
339 void *data;
340 int ret;
341
342 data = erofs_read_metabuf(&buf, sb, 0, EROFS_KMAP);
343 if (IS_ERR(data)) {
344 erofs_err(sb, "cannot read erofs superblock");
345 return PTR_ERR(data);
346 }
347
348 sbi = EROFS_SB(sb);
349 dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);
350
351 ret = -EINVAL;
352 if (le32_to_cpu(dsb->magic) != EROFS_SUPER_MAGIC_V1) {
353 erofs_err(sb, "cannot find valid erofs superblock");
354 goto out;
355 }
356
357 sbi->blkszbits = dsb->blkszbits;
358 if (sbi->blkszbits < 9 || sbi->blkszbits > PAGE_SHIFT) {
359 erofs_err(sb, "blkszbits %u isn't supported", sbi->blkszbits);
360 goto out;
361 }
362 if (dsb->dirblkbits) {
363 erofs_err(sb, "dirblkbits %u isn't supported", dsb->dirblkbits);
364 goto out;
365 }
366
367 sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
368 if (erofs_sb_has_sb_chksum(sbi)) {
369 ret = erofs_superblock_csum_verify(sb, data);
370 if (ret)
371 goto out;
372 }
373
374 ret = -EINVAL;
375 if (!check_layout_compatibility(sb, dsb))
376 goto out;
377
378 sbi->sb_size = 128 + dsb->sb_extslots * EROFS_SB_EXTSLOT_SIZE;
379 if (sbi->sb_size > PAGE_SIZE - EROFS_SUPER_OFFSET) {
380 erofs_err(sb, "invalid sb_extslots %u (more than a fs block)",
381 sbi->sb_size);
382 goto out;
383 }
384 sbi->primarydevice_blocks = le32_to_cpu(dsb->blocks);
385 sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
386 #ifdef CONFIG_EROFS_FS_XATTR
387 sbi->xattr_blkaddr = le32_to_cpu(dsb->xattr_blkaddr);
388 #endif
389 sbi->islotbits = ilog2(sizeof(struct erofs_inode_compact));
390 sbi->root_nid = le16_to_cpu(dsb->root_nid);
391 sbi->packed_nid = le64_to_cpu(dsb->packed_nid);
392 sbi->inos = le64_to_cpu(dsb->inos);
393
> 394 sbi->xattr_prefix_start = le32_to_cpu(dsb->xattr_prefix_start);
> 395 sbi->xattr_prefix_count = dsb->xattr_prefix_count;
396
397 sbi->build_time = le64_to_cpu(dsb->build_time);
398 sbi->build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
399
400 memcpy(&sb->s_uuid, dsb->uuid, sizeof(dsb->uuid));
401
402 ret = strscpy(sbi->volume_name, dsb->volume_name,
403 sizeof(dsb->volume_name));
404 if (ret < 0) { /* -E2BIG */
405 erofs_err(sb, "bad volume name without NIL terminator");
406 ret = -EFSCORRUPTED;
407 goto out;
408 }
409
410 /* parse on-disk compression configurations */
411 if (erofs_sb_has_compr_cfgs(sbi))
412 ret = erofs_load_compr_cfgs(sb, dsb);
413 else
414 ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
415 if (ret < 0)
416 goto out;
417
418 /* handle multiple devices */
419 ret = erofs_scan_devices(sb, dsb);
420
421 if (erofs_sb_has_ztailpacking(sbi))
422 erofs_info(sb, "EXPERIMENTAL compressed inline data feature in use. Use at your own risk!");
423 if (erofs_is_fscache_mode(sb))
424 erofs_info(sb, "EXPERIMENTAL fscache-based on-demand read feature in use. Use at your own risk!");
425 if (erofs_sb_has_fragments(sbi))
426 erofs_info(sb, "EXPERIMENTAL compressed fragments feature in use. Use at your own risk!");
427 if (erofs_sb_has_dedupe(sbi))
428 erofs_info(sb, "EXPERIMENTAL global deduplication feature in use. Use at your own risk!");
429 out:
430 erofs_put_metabuf(&buf);
431 return ret;
432 }
433

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-04-07 22:40:05

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH v2 7/7] erofs: enable long extended attribute name prefixes

Let's enable long xattr name prefix feature. Old kernels will just
ignore / skip such extended attributes so that in case you don't want
to mount such images. Add another incompatible feature as an option
for this.

Signed-off-by: Jingbo Xu <[email protected]>
---
v2: fix build error when CONFIG_EROFS_FS_XATTR is not defined
---
fs/erofs/erofs_fs.h | 4 +++-
fs/erofs/internal.h | 1 +
fs/erofs/super.c | 7 +++++++
3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index ea62f83dac40..ac42a7255b39 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -27,6 +27,7 @@
#define EROFS_FEATURE_INCOMPAT_ZTAILPACKING 0x00000010
#define EROFS_FEATURE_INCOMPAT_FRAGMENTS 0x00000020
#define EROFS_FEATURE_INCOMPAT_DEDUPE 0x00000020
+#define EROFS_FEATURE_INCOMPAT_XATTR_PREFIXES 0x00000040
#define EROFS_ALL_FEATURE_INCOMPAT \
(EROFS_FEATURE_INCOMPAT_ZERO_PADDING | \
EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
@@ -36,7 +37,8 @@
EROFS_FEATURE_INCOMPAT_COMPR_HEAD2 | \
EROFS_FEATURE_INCOMPAT_ZTAILPACKING | \
EROFS_FEATURE_INCOMPAT_FRAGMENTS | \
- EROFS_FEATURE_INCOMPAT_DEDUPE)
+ EROFS_FEATURE_INCOMPAT_DEDUPE | \
+ EROFS_FEATURE_INCOMPAT_XATTR_PREFIXES)

#define EROFS_SB_EXTSLOT_SIZE 16

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 5a9c19654b19..f675050af2bb 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -285,6 +285,7 @@ EROFS_FEATURE_FUNCS(compr_head2, incompat, INCOMPAT_COMPR_HEAD2)
EROFS_FEATURE_FUNCS(ztailpacking, incompat, INCOMPAT_ZTAILPACKING)
EROFS_FEATURE_FUNCS(fragments, incompat, INCOMPAT_FRAGMENTS)
EROFS_FEATURE_FUNCS(dedupe, incompat, INCOMPAT_DEDUPE)
+EROFS_FEATURE_FUNCS(xattr_prefixes, incompat, INCOMPAT_XATTR_PREFIXES)
EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)

/* atomic flag definitions */
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index bf396e0c243a..e44cd69c9d9c 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -385,6 +385,8 @@ static int erofs_read_superblock(struct super_block *sb)
sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
#ifdef CONFIG_EROFS_FS_XATTR
sbi->xattr_blkaddr = le32_to_cpu(dsb->xattr_blkaddr);
+ sbi->xattr_prefix_start = le32_to_cpu(dsb->xattr_prefix_start);
+ sbi->xattr_prefix_count = dsb->xattr_prefix_count;
#endif
sbi->islotbits = ilog2(sizeof(struct erofs_inode_compact));
sbi->root_nid = le16_to_cpu(dsb->root_nid);
@@ -822,6 +824,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
if (err)
return err;

+ err = erofs_xattr_prefixes_init(sb);
+ if (err)
+ return err;
+
err = erofs_register_sysfs(sb);
if (err)
return err;
@@ -981,6 +987,7 @@ static void erofs_put_super(struct super_block *sb)

erofs_unregister_sysfs(sb);
erofs_shrinker_unregister(sb);
+ erofs_xattr_prefixes_cleanup(sb);
#ifdef CONFIG_EROFS_FS_ZIP
iput(sbi->managed_cache);
sbi->managed_cache = NULL;
--
2.19.1.6.gb485710b

2023-04-09 10:54:53

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH 3/7] erofs: move packed inode out of the compression part



On 2023/4/7 22:17, Jingbo Xu wrote:
> packed inode could be used in more scenarios which are independent of
> compression in the future.
>
> For example, packed inode could be used to keep extra long xattr
> prefixes with the help of following patches.
>
> Signed-off-by: Jingbo Xu <[email protected]>

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

2023-04-10 03:17:44

by Yue Hu

[permalink] [raw]
Subject: Re: [PATCH 3/7] erofs: move packed inode out of the compression part

On Fri, 7 Apr 2023 22:17:06 +0800
Jingbo Xu <[email protected]> wrote:

> packed inode could be used in more scenarios which are independent of
> compression in the future.
>
> For example, packed inode could be used to keep extra long xattr
> prefixes with the help of following patches.
>
> Signed-off-by: Jingbo Xu <[email protected]>

Reviewed-by: Yue Hu <[email protected]>

> ---
> fs/erofs/internal.h | 2 +-
> fs/erofs/super.c | 4 +---
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index caea9dc1cd82..8b5168f94dd2 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -134,8 +134,8 @@ struct erofs_sb_info {
> struct inode *managed_cache;
>
> struct erofs_sb_lz4_info lz4;
> - struct inode *packed_inode;
> #endif /* CONFIG_EROFS_FS_ZIP */
> + struct inode *packed_inode;
> struct erofs_dev_context *devs;
> struct dax_device *dax_dev;
> u64 dax_part_off;
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 325602820dc8..8f2f8433db61 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -810,7 +810,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>
> erofs_shrinker_register(sb);
> /* sb->s_umount is already locked, SB_ACTIVE and SB_BORN are not set */
> -#ifdef CONFIG_EROFS_FS_ZIP
> if (erofs_sb_has_fragments(sbi) && sbi->packed_nid) {
> sbi->packed_inode = erofs_iget(sb, sbi->packed_nid);
> if (IS_ERR(sbi->packed_inode)) {
> @@ -819,7 +818,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
> return err;
> }
> }
> -#endif
> err = erofs_init_managed_cache(sb);
> if (err)
> return err;
> @@ -986,9 +984,9 @@ static void erofs_put_super(struct super_block *sb)
> #ifdef CONFIG_EROFS_FS_ZIP
> iput(sbi->managed_cache);
> sbi->managed_cache = NULL;
> +#endif
> iput(sbi->packed_inode);
> sbi->packed_inode = NULL;
> -#endif
> erofs_free_dev_context(sbi->devs);
> sbi->devs = NULL;
> erofs_fscache_unregister_fs(sb);

2023-04-10 05:44:21

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes



On 2023/4/7 22:17, Jingbo Xu wrote:
> Besides the predefined xattr name prefixes, introduces long xattr name
> prefixes, which work similarly as the predefined name prefixes, except
> that they are user specified.
>
> It is especially useful for use cases together with overlayfs like
> Composefs model, which introduces diverse xattr values with only a few
> common xattr names (trusted.overlay.redirect, trusted.overlay.digest,
> and maybe more in the future). That makes the existing predefined
> prefixes ineffective in both image size and runtime performance.
>
> When a user specified long xattr name prefix is used, only the trailing
> part of the xattr name apart from the long xattr name prefix will be
> stored in erofs_xattr_entry.e_name. e_name is empty if the xattr name
> matches exactly as the long xattr name prefix. All long xattr prefixes
> are stored in the packed or meta inode, which depends if fragments
> feature is enabled or not.
>
> For each long xattr name prefix, the on-disk format is kept as the same
> as the unique metadata format: ALIGN({__le16 len, data}, 4), where len
> represents the total size of struct erofs_xattr_long_prefix, followed
> by data of struct erofs_xattr_long_prefix itself.
>
> Each erofs_xattr_long_prefix keeps predefined prefixes (base_index)
> and the remaining prefix string without the trailing '\0'.
>
> Two fields are introduced to the on-disk superblock, where
> xattr_prefix_count represents the total number of the long xattr name
> prefixes recorded, and xattr_prefix_start represents the start offset of
> recorded name prefixes in the packed/meta inode divided by 4.
>
> When referring to a long xattr name prefix, the highest bit (bit 7) of
> erofs_xattr_entry.e_name_index is set, while the lower bits (bit 0-6)
> as a whole represents the index of the referred long name prefix among
> all long xattr name prefixes.
>
> Signed-off-by: Jingbo Xu <[email protected]>

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

> ---
> fs/erofs/erofs_fs.h | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
> index 44876a97cabd..ea62f83dac40 100644
> --- a/fs/erofs/erofs_fs.h
> +++ b/fs/erofs/erofs_fs.h
> @@ -76,7 +76,8 @@ struct erofs_super_block {
> __le16 extra_devices; /* # of devices besides the primary device */
> __le16 devt_slotoff; /* startoff = devt_slotoff * devt_slotsize */
> __u8 dirblkbits; /* directory block size in bit shift */
> - __u8 reserved[5];
> + __u8 xattr_prefix_count; /* # of long xattr name prefixes */
> + __le32 xattr_prefix_start; /* start of long xattr prefixes */
> __le64 packed_nid; /* nid of the special packed inode */
> __u8 reserved2[24];
> };
> @@ -229,6 +230,13 @@ struct erofs_xattr_ibody_header {
> #define EROFS_XATTR_INDEX_LUSTRE 5
> #define EROFS_XATTR_INDEX_SECURITY 6
>
> +/*
> + * bit 7 of e_name_index is set when it refers to a long xattr name prefix,
> + * while the remained lower bits represent the index of the prefix.
> + */
> +#define EROFS_XATTR_LONG_PREFIX 0x80
> +#define EROFS_XATTR_LONG_PREFIX_MASK 0x7f
> +
> /* xattr entry (for both inline & shared xattrs) */
> struct erofs_xattr_entry {
> __u8 e_name_len; /* length of name */
> @@ -238,6 +246,12 @@ struct erofs_xattr_entry {
> char e_name[]; /* attribute name */
> };
>
> +/* long xattr name prefix */
> +struct erofs_xattr_long_prefix {
> + __u8 base_index; /* short xattr name prefix index */
> + char infix[]; /* infix apart from short prefix */
> +};
> +
> static inline unsigned int erofs_xattr_ibody_size(__le16 i_xattr_icount)
> {
> if (!i_xattr_icount)

2023-04-10 05:49:04

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH 5/7] erofs: add helpers to load long xattr name prefixes



On 2023/4/7 22:17, Jingbo Xu wrote:
> Long xattr name prefixes will be scanned upon mounting and the in-memory
> long xattr name prefix array will be initialized accordingly.
>
> Signed-off-by: Jingbo Xu <[email protected]>

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

2023-04-10 06:06:55

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v2 7/7] erofs: enable long extended attribute name prefixes



On 2023/4/8 06:28, Jingbo Xu wrote:
> Let's enable long xattr name prefix feature. Old kernels will just
> ignore / skip such extended attributes so that in case you don't want
> to mount such images. Add another incompatible feature as an option
> for this.
>
> Signed-off-by: Jingbo Xu <[email protected]>
> ---
> v2: fix build error when CONFIG_EROFS_FS_XATTR is not defined

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

2023-04-10 06:10:15

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH 6/7] erofs: handle long xattr name prefixes properly



On 2023/4/7 22:17, Jingbo Xu wrote:
> Make .{list,get}xattr routines adapted to long xattr name prefixes.
> When the bit 7 of erofs_xattr_entry.e_name_index is set, it indicates
> that it refers to a long xattr name prefix.
>
> Signed-off-by: Jingbo Xu <[email protected]>
> ---
> fs/erofs/xattr.c | 60 +++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index 684571e83a2c..8d81593655e8 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -301,11 +301,39 @@ struct getxattr_iter {
> struct qstr name;
> };
>
> +static int erofs_xattr_long_entrymatch(struct getxattr_iter *it,
> + struct erofs_xattr_entry *entry)
> +{
> + struct erofs_sb_info *sbi = EROFS_SB(it->it.sb);
> + u8 idx = entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK;
> + struct erofs_xattr_prefix_item *pf;
> +
> + if (idx >= sbi->xattr_prefix_count)
> + return -ENOATTR;
> +
> + pf = &sbi->xattr_prefixes[idx];

struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes + idx;

if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
return -ENOATTR;

?


> + if (it->index != pf->prefix->base_index)
> + return -ENOATTR;
> +
> + if (strncmp(it->name.name, pf->prefix->infix, pf->infix_len))
> + return -ENOATTR;
> +
> + it->name.name += pf->infix_len;
> + it->name.len -= pf->infix_len;
> + if (it->name.len != entry->e_name_len)
> + return -ENOATTR;
> + return 0;
> +}
> +
> static int xattr_entrymatch(struct xattr_iter *_it,
> struct erofs_xattr_entry *entry)
> {
> struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
>
> + /* should also match the infix for long name prefixes */
> + if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX)
> + return erofs_xattr_long_entrymatch(it, entry);
> +
> return (it->index != entry->e_name_index ||
> it->name.len != entry->e_name_len) ? -ENOATTR : 0;
> }
> @@ -487,12 +515,26 @@ static int xattr_entrylist(struct xattr_iter *_it,
> {
> struct listxattr_iter *it =
> container_of(_it, struct listxattr_iter, it);
> - unsigned int prefix_len;
> - const char *prefix;
> -
> - const struct xattr_handler *h =
> - erofs_xattr_handler(entry->e_name_index);
> + unsigned int base_index = entry->e_name_index;
> + unsigned int prefix_len, infix_len = 0;
> + const char *prefix, *infix = NULL;
> + const struct xattr_handler *h;
> +
> + if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) {
> + struct erofs_sb_info *sbi = EROFS_SB(_it->sb);
> + u8 idx = entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK;
> + struct erofs_xattr_prefix_item *pf;
> +
> + if (idx >= sbi->xattr_prefix_count)
> + return 1;
> +
> + pf = &sbi->xattr_prefixes[idx];

struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes + idx;

if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
return 1;
?


Otherwise it looks good to me,
Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang


> + infix = pf->prefix->infix;
> + infix_len = pf->infix_len;
> + base_index = pf->prefix->base_index;
> + }
>
> + h = erofs_xattr_handler(base_index);
> if (!h || (h->list && !h->list(it->dentry)))
> return 1;
>
> @@ -500,16 +542,18 @@ static int xattr_entrylist(struct xattr_iter *_it,
> prefix_len = strlen(prefix);
>
> if (!it->buffer) {
> - it->buffer_ofs += prefix_len + entry->e_name_len + 1;
> + it->buffer_ofs += prefix_len + infix_len +
> + entry->e_name_len + 1;
> return 1;
> }
>
> - if (it->buffer_ofs + prefix_len
> + if (it->buffer_ofs + prefix_len + infix_len +
> + entry->e_name_len + 1 > it->buffer_size)
> return -ERANGE;
>
> memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
> - it->buffer_ofs += prefix_len;
> + memcpy(it->buffer + it->buffer_ofs + prefix_len, infix, infix_len);
> + it->buffer_ofs += prefix_len + infix_len;
> return 0;
> }
>

2023-04-10 06:44:32

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH v2 6/7] erofs: handle long xattr name prefixes properly

Make .{list,get}xattr routines adapted to long xattr name prefixes.
When the bit 7 of erofs_xattr_entry.e_name_index is set, it indicates
that it refers to a long xattr name prefix.

Signed-off-by: Jingbo Xu <[email protected]>
---
v2: remove "idx" temporary variable (Gao Xiang)
---
fs/erofs/xattr.c | 57 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 684571e83a2c..4ad290b6acb7 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -301,11 +301,38 @@ struct getxattr_iter {
struct qstr name;
};

+static int erofs_xattr_long_entrymatch(struct getxattr_iter *it,
+ struct erofs_xattr_entry *entry)
+{
+ struct erofs_sb_info *sbi = EROFS_SB(it->it.sb);
+ struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
+ (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
+
+ if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
+ return -ENOATTR;
+
+ if (it->index != pf->prefix->base_index)
+ return -ENOATTR;
+
+ if (strncmp(it->name.name, pf->prefix->infix, pf->infix_len))
+ return -ENOATTR;
+
+ it->name.name += pf->infix_len;
+ it->name.len -= pf->infix_len;
+ if (it->name.len != entry->e_name_len)
+ return -ENOATTR;
+ return 0;
+}
+
static int xattr_entrymatch(struct xattr_iter *_it,
struct erofs_xattr_entry *entry)
{
struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);

+ /* should also match the infix for long name prefixes */
+ if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX)
+ return erofs_xattr_long_entrymatch(it, entry);
+
return (it->index != entry->e_name_index ||
it->name.len != entry->e_name_len) ? -ENOATTR : 0;
}
@@ -487,12 +514,24 @@ static int xattr_entrylist(struct xattr_iter *_it,
{
struct listxattr_iter *it =
container_of(_it, struct listxattr_iter, it);
- unsigned int prefix_len;
- const char *prefix;
-
- const struct xattr_handler *h =
- erofs_xattr_handler(entry->e_name_index);
+ unsigned int base_index = entry->e_name_index;
+ unsigned int prefix_len, infix_len = 0;
+ const char *prefix, *infix = NULL;
+ const struct xattr_handler *h;
+
+ if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) {
+ struct erofs_sb_info *sbi = EROFS_SB(_it->sb);
+ struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
+ (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
+
+ if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
+ return 1;
+ infix = pf->prefix->infix;
+ infix_len = pf->infix_len;
+ base_index = pf->prefix->base_index;
+ }

+ h = erofs_xattr_handler(base_index);
if (!h || (h->list && !h->list(it->dentry)))
return 1;

@@ -500,16 +539,18 @@ static int xattr_entrylist(struct xattr_iter *_it,
prefix_len = strlen(prefix);

if (!it->buffer) {
- it->buffer_ofs += prefix_len + entry->e_name_len + 1;
+ it->buffer_ofs += prefix_len + infix_len +
+ entry->e_name_len + 1;
return 1;
}

- if (it->buffer_ofs + prefix_len
+ if (it->buffer_ofs + prefix_len + infix_len +
+ entry->e_name_len + 1 > it->buffer_size)
return -ERANGE;

memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
- it->buffer_ofs += prefix_len;
+ memcpy(it->buffer + it->buffer_ofs + prefix_len, infix, infix_len);
+ it->buffer_ofs += prefix_len + infix_len;
return 0;
}

--
2.19.1.6.gb485710b

2023-04-10 06:50:16

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] erofs: handle long xattr name prefixes properly



On 2023/4/10 14:39, Jingbo Xu wrote:
> Make .{list,get}xattr routines adapted to long xattr name prefixes.
> When the bit 7 of erofs_xattr_entry.e_name_index is set, it indicates
> that it refers to a long xattr name prefix.
>
> Signed-off-by: Jingbo Xu <[email protected]>
> ---
> v2: remove "idx" temporary variable (Gao Xiang)

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

2023-04-11 09:40:08

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH v3 6/7] erofs: handle long xattr name prefixes properly

Make .{list,get}xattr routines adapted to long xattr name prefixes.
When the bit 7 of erofs_xattr_entry.e_name_index is set, it indicates
that it refers to a long xattr name prefix.

Signed-off-by: Jingbo Xu <[email protected]>
---
v3: introduce infix_len to struct getxattr_iter, and refactor the
implementation of xattr_entrymatch(), erofs_xattr_long_entrymatch(), and
xattr_namematch() accordingly.

The erofs_xattr_long_entrymatch() of v2 version will advance
it->name.name pointer by pf->infix_len prematurely, as the following
xattr_namematch() may fail (-ENOATTR) since mismatching. And then
it->name.name will be compared with the next xattr entry, while
it->name.name has been mistakenly modified in the previous round. This
will cause -ENOATTR error on the existing xattr.

---
fs/erofs/xattr.c | 68 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 56 insertions(+), 12 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 684571e83a2c..a04724c816e5 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -297,17 +297,45 @@ struct getxattr_iter {
struct xattr_iter it;

char *buffer;
- int buffer_size, index;
+ int buffer_size, index, infix_len;
struct qstr name;
};

+static int erofs_xattr_long_entrymatch(struct getxattr_iter *it,
+ struct erofs_xattr_entry *entry)
+{
+ struct erofs_sb_info *sbi = EROFS_SB(it->it.sb);
+ struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
+ (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
+
+ if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
+ return -ENOATTR;
+
+ if (it->index != pf->prefix->base_index ||
+ it->name.len != entry->e_name_len + pf->infix_len)
+ return -ENOATTR;
+
+ if (memcmp(it->name.name, pf->prefix->infix, pf->infix_len))
+ return -ENOATTR;
+
+ it->infix_len = pf->infix_len;
+ return 0;
+}
+
static int xattr_entrymatch(struct xattr_iter *_it,
struct erofs_xattr_entry *entry)
{
struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);

- return (it->index != entry->e_name_index ||
- it->name.len != entry->e_name_len) ? -ENOATTR : 0;
+ /* should also match the infix for long name prefixes */
+ if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX)
+ return erofs_xattr_long_entrymatch(it, entry);
+
+ if (it->index != entry->e_name_index ||
+ it->name.len != entry->e_name_len)
+ return -ENOATTR;
+ it->infix_len = 0;
+ return 0;
}

static int xattr_namematch(struct xattr_iter *_it,
@@ -315,7 +343,9 @@ static int xattr_namematch(struct xattr_iter *_it,
{
struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);

- return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 0;
+ if (memcmp(buf, it->name.name + it->infix_len + processed, len))
+ return -ENOATTR;
+ return 0;
}

static int xattr_checkbuffer(struct xattr_iter *_it,
@@ -487,12 +517,24 @@ static int xattr_entrylist(struct xattr_iter *_it,
{
struct listxattr_iter *it =
container_of(_it, struct listxattr_iter, it);
- unsigned int prefix_len;
- const char *prefix;
-
- const struct xattr_handler *h =
- erofs_xattr_handler(entry->e_name_index);
+ unsigned int base_index = entry->e_name_index;
+ unsigned int prefix_len, infix_len = 0;
+ const char *prefix, *infix = NULL;
+ const struct xattr_handler *h;
+
+ if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) {
+ struct erofs_sb_info *sbi = EROFS_SB(_it->sb);
+ struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
+ (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
+
+ if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
+ return 1;
+ infix = pf->prefix->infix;
+ infix_len = pf->infix_len;
+ base_index = pf->prefix->base_index;
+ }

+ h = erofs_xattr_handler(base_index);
if (!h || (h->list && !h->list(it->dentry)))
return 1;

@@ -500,16 +542,18 @@ static int xattr_entrylist(struct xattr_iter *_it,
prefix_len = strlen(prefix);

if (!it->buffer) {
- it->buffer_ofs += prefix_len + entry->e_name_len + 1;
+ it->buffer_ofs += prefix_len + infix_len +
+ entry->e_name_len + 1;
return 1;
}

- if (it->buffer_ofs + prefix_len
+ if (it->buffer_ofs + prefix_len + infix_len +
+ entry->e_name_len + 1 > it->buffer_size)
return -ERANGE;

memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
- it->buffer_ofs += prefix_len;
+ memcpy(it->buffer + it->buffer_ofs + prefix_len, infix, infix_len);
+ it->buffer_ofs += prefix_len + infix_len;
return 0;
}

--
2.19.1.6.gb485710b

2023-04-11 09:42:08

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] erofs: handle long xattr name prefixes properly



On 2023/4/11 17:35, Jingbo Xu wrote:
> Make .{list,get}xattr routines adapted to long xattr name prefixes.
> When the bit 7 of erofs_xattr_entry.e_name_index is set, it indicates
> that it refers to a long xattr name prefix.
>
> Signed-off-by: Jingbo Xu <[email protected]>
> ---
> v3: introduce infix_len to struct getxattr_iter, and refactor the
> implementation of xattr_entrymatch(), erofs_xattr_long_entrymatch(), and
> xattr_namematch() accordingly.
>
> The erofs_xattr_long_entrymatch() of v2 version will advance
> it->name.name pointer by pf->infix_len prematurely, as the following
> xattr_namematch() may fail (-ENOATTR) since mismatching. And then
> it->name.name will be compared with the next xattr entry, while
> it->name.name has been mistakenly modified in the previous round. This
> will cause -ENOATTR error on the existing xattr.

Yes, please also help add a new erofs-utils testcase for this.

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

>
> ---
> fs/erofs/xattr.c | 68 +++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 56 insertions(+), 12 deletions(-)
>
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index 684571e83a2c..a04724c816e5 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -297,17 +297,45 @@ struct getxattr_iter {
> struct xattr_iter it;
>
> char *buffer;
> - int buffer_size, index;
> + int buffer_size, index, infix_len;
> struct qstr name;
> };
>
> +static int erofs_xattr_long_entrymatch(struct getxattr_iter *it,
> + struct erofs_xattr_entry *entry)
> +{
> + struct erofs_sb_info *sbi = EROFS_SB(it->it.sb);
> + struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
> + (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
> +
> + if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
> + return -ENOATTR;
> +
> + if (it->index != pf->prefix->base_index ||
> + it->name.len != entry->e_name_len + pf->infix_len)
> + return -ENOATTR;
> +
> + if (memcmp(it->name.name, pf->prefix->infix, pf->infix_len))
> + return -ENOATTR;
> +
> + it->infix_len = pf->infix_len;
> + return 0;
> +}
> +
> static int xattr_entrymatch(struct xattr_iter *_it,
> struct erofs_xattr_entry *entry)
> {
> struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
>
> - return (it->index != entry->e_name_index ||
> - it->name.len != entry->e_name_len) ? -ENOATTR : 0;
> + /* should also match the infix for long name prefixes */
> + if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX)
> + return erofs_xattr_long_entrymatch(it, entry);
> +
> + if (it->index != entry->e_name_index ||
> + it->name.len != entry->e_name_len)
> + return -ENOATTR;
> + it->infix_len = 0;
> + return 0;
> }
>
> static int xattr_namematch(struct xattr_iter *_it,
> @@ -315,7 +343,9 @@ static int xattr_namematch(struct xattr_iter *_it,
> {
> struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
>
> - return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 0;
> + if (memcmp(buf, it->name.name + it->infix_len + processed, len))
> + return -ENOATTR;
> + return 0;
> }
>
> static int xattr_checkbuffer(struct xattr_iter *_it,
> @@ -487,12 +517,24 @@ static int xattr_entrylist(struct xattr_iter *_it,
> {
> struct listxattr_iter *it =
> container_of(_it, struct listxattr_iter, it);
> - unsigned int prefix_len;
> - const char *prefix;
> -
> - const struct xattr_handler *h =
> - erofs_xattr_handler(entry->e_name_index);
> + unsigned int base_index = entry->e_name_index;
> + unsigned int prefix_len, infix_len = 0;
> + const char *prefix, *infix = NULL;
> + const struct xattr_handler *h;
> +
> + if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) {
> + struct erofs_sb_info *sbi = EROFS_SB(_it->sb);
> + struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
> + (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
> +
> + if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
> + return 1;
> + infix = pf->prefix->infix;
> + infix_len = pf->infix_len;
> + base_index = pf->prefix->base_index;
> + }
>
> + h = erofs_xattr_handler(base_index);
> if (!h || (h->list && !h->list(it->dentry)))
> return 1;
>
> @@ -500,16 +542,18 @@ static int xattr_entrylist(struct xattr_iter *_it,
> prefix_len = strlen(prefix);
>
> if (!it->buffer) {
> - it->buffer_ofs += prefix_len + entry->e_name_len + 1;
> + it->buffer_ofs += prefix_len + infix_len +
> + entry->e_name_len + 1;
> return 1;
> }
>
> - if (it->buffer_ofs + prefix_len
> + if (it->buffer_ofs + prefix_len + infix_len +
> + entry->e_name_len + 1 > it->buffer_size)
> return -ERANGE;
>
> memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
> - it->buffer_ofs += prefix_len;
> + memcpy(it->buffer + it->buffer_ofs + prefix_len, infix, infix_len);
> + it->buffer_ofs += prefix_len + infix_len;
> return 0;
> }
>

2023-04-16 14:29:52

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH 0/7] erofs: introduce long xattr name prefixes feature

On 2023/4/7 22:17, Jingbo Xu wrote:
> Background
> =========
> overlayfs uses xattrs to keep its own metadata. If such xattrs are
> heavily used, such as Composefs model [1], large amount of xattrs
> with diverse xattr values exist but only a few common xattr names
> are valid (trusted.overlay.redirect, trusted.overlay.digest, and
> maybe more in the future) . For example:
>
> # file 1
> trusted.overlay.redirect="xxx"
>
> # file 2
> trusted.overlay.redirect="yyy"
>
> That makes the existing predefined prefixes ineffective in both image
> size and runtime performance.
>
> Solution Proposed
> ====================
> Let's introduce long xattr name prefixes now to fix this. They work
> similarly as the predefined name prefixes, except that long xattr name
> prefixes are user specified.
>
> When a long xattr name prefix is used, the shared long xattr prefixes
> are stored in the packed or meta inode, while the remained trailing part
> of the xattr name apart from the long xattr name prefix will be stored
> in erofs_xattr_entry.e_name. e_name is empty if the xattr name matches
> exactly as the long xattr name prefix.
>
> Erofs image sizes will be smaller in the above described scenario where
> all files have diverse xattr values with the same set of xattr names[2],
> such as having following xattrs like:
>
> trusted.overlay.metacopy=""
> trusted.overlay.redirect="xxx"
>
> Here are the image sizes for comparison (32-byte inodes are used):
>
> ```
> 7.4M large.erofs.T0.xattr
> 6.4M large.erofs.T0.xattr.share
> ```
>
> - share: "--xattr-prefix=trusted.overlay.redirect" option of mkfs.erofs.
> w/ this option, the long xattr name prefixes feature is enabled.
>
> It can be seen ~14% disk space is saved with this feature in this
> typical workload, therefore metadata I/Os could also be more effective
> then.
>
> Test
> ====
> It passes erofs/019 of erofs-utils.
>
>
> [1] https://lore.kernel.org/all/CAOQ4uxgGc33_QVBXMbQTnmbpHio4amv=W7ax2vQ1UMet0k_KoA@mail.gmail.com/
> [2] https://my.owndrive.com/index.php/s/irHJXRpZHtT3a5i
>
>
>
> Gao Xiang (1):
> erofs: keep meta inode into erofs_buf
>
> Jingbo Xu (6):
> erofs: initialize packed inode after root inode is assigned
> erofs: move packed inode out of the compression part
> erofs: introduce on-disk format for long xattr name prefixes
> erofs: add helpers to load long xattr name prefixes
> erofs: handle long xattr name prefixes properly
> erofs: enable long extended attribute name prefixes

Acked-by: Chao Yu <[email protected]>

Thanks,