linux-erofs.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] erofs: introduce long xattr name prefixes feature
@ 2023-04-07 14:17 Jingbo Xu
  2023-04-07 14:17 ` [PATCH 1/7] erofs: keep meta inode into erofs_buf Jingbo Xu
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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


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

* [PATCH 1/7] erofs: keep meta inode into erofs_buf
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
@ 2023-04-07 14:17 ` Jingbo Xu
  2023-04-07 14:17 ` [PATCH 2/7] erofs: initialize packed inode after root inode is assigned Jingbo Xu
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

From: Gao Xiang <hsiangkao@linux.alibaba.com>

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

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 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


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

* [PATCH 2/7] erofs: initialize packed inode after root inode is assigned
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
  2023-04-07 14:17 ` [PATCH 1/7] erofs: keep meta inode into erofs_buf Jingbo Xu
@ 2023-04-07 14:17 ` Jingbo Xu
  2023-04-09 10:52   ` Gao Xiang
  2023-04-10  2:44   ` Yue Hu
  2023-04-07 14:17 ` [PATCH 3/7] erofs: move packed inode out of the compression part Jingbo Xu
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

As commit 8f7acdae2cd4 ("staging: erofs: kill all failure handling in
fill_super()"), move the initialization of packed inode after root
inode is assigned, so that the iput() in .put_super() is adequate as
the failure handling.

Otherwise, iput() is also needed in .kill_sb(), in case of the mounting
fails halfway.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
 fs/erofs/internal.h |  1 +
 fs/erofs/super.c    | 22 +++++++++++-----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 2bcff3194e4a..caea9dc1cd82 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -157,6 +157,7 @@ struct erofs_sb_info {
 
 	/* what we really care is nid, rather than ino.. */
 	erofs_nid_t root_nid;
+	erofs_nid_t packed_nid;
 	/* used for statfs, f_files - f_favail */
 	u64 inos;
 
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 58ffbf410bfb..325602820dc8 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -388,17 +388,7 @@ static int erofs_read_superblock(struct super_block *sb)
 #endif
 	sbi->islotbits = ilog2(sizeof(struct erofs_inode_compact));
 	sbi->root_nid = le16_to_cpu(dsb->root_nid);
-#ifdef CONFIG_EROFS_FS_ZIP
-	sbi->packed_inode = NULL;
-	if (erofs_sb_has_fragments(sbi) && dsb->packed_nid) {
-		sbi->packed_inode =
-			erofs_iget(sb, le64_to_cpu(dsb->packed_nid));
-		if (IS_ERR(sbi->packed_inode)) {
-			ret = PTR_ERR(sbi->packed_inode);
-			goto out;
-		}
-	}
-#endif
+	sbi->packed_nid = le64_to_cpu(dsb->packed_nid);
 	sbi->inos = le64_to_cpu(dsb->inos);
 
 	sbi->build_time = le64_to_cpu(dsb->build_time);
@@ -820,6 +810,16 @@ 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)) {
+			err = PTR_ERR(sbi->packed_inode);
+			sbi->packed_inode = NULL;
+			return err;
+		}
+	}
+#endif
 	err = erofs_init_managed_cache(sb);
 	if (err)
 		return err;
-- 
2.19.1.6.gb485710b


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

* [PATCH 3/7] erofs: move packed inode out of the compression part
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
  2023-04-07 14:17 ` [PATCH 1/7] erofs: keep meta inode into erofs_buf Jingbo Xu
  2023-04-07 14:17 ` [PATCH 2/7] erofs: initialize packed inode after root inode is assigned Jingbo Xu
@ 2023-04-07 14:17 ` Jingbo Xu
  2023-04-09 10:53   ` Gao Xiang
  2023-04-10  2:45   ` Yue Hu
  2023-04-07 14:17 ` [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes Jingbo Xu
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
 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


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

* [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
                   ` (2 preceding siblings ...)
  2023-04-07 14:17 ` [PATCH 3/7] erofs: move packed inode out of the compression part Jingbo Xu
@ 2023-04-07 14:17 ` Jingbo Xu
  2023-04-10  5:24   ` Gao Xiang
  2023-04-07 14:17 ` [PATCH 5/7] erofs: add helpers to load " Jingbo Xu
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
 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


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

* [PATCH 5/7] erofs: add helpers to load long xattr name prefixes
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
                   ` (3 preceding siblings ...)
  2023-04-07 14:17 ` [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes Jingbo Xu
@ 2023-04-07 14:17 ` Jingbo Xu
  2023-04-10  5:44   ` Gao Xiang
  2023-04-07 14:17 ` [PATCH 6/7] erofs: handle long xattr name prefixes properly Jingbo Xu
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
 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


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

* [PATCH 6/7] erofs: handle long xattr name prefixes properly
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
                   ` (4 preceding siblings ...)
  2023-04-07 14:17 ` [PATCH 5/7] erofs: add helpers to load " Jingbo Xu
@ 2023-04-07 14:17 ` Jingbo Xu
  2023-04-10  5:53   ` Gao Xiang
  2023-04-10  6:39   ` [PATCH v2 " Jingbo Xu
  2023-04-07 14:17 ` [PATCH 7/7] erofs: enable long extended attribute name prefixes Jingbo Xu
  2023-04-16 14:24 ` [PATCH 0/7] erofs: introduce long xattr name prefixes feature Chao Yu
  7 siblings, 2 replies; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
 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


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

* [PATCH 7/7] erofs: enable long extended attribute name prefixes
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
                   ` (5 preceding siblings ...)
  2023-04-07 14:17 ` [PATCH 6/7] erofs: handle long xattr name prefixes properly Jingbo Xu
@ 2023-04-07 14:17 ` Jingbo Xu
  2023-04-07 17:29   ` kernel test robot
                     ` (2 more replies)
  2023-04-16 14:24 ` [PATCH 0/7] erofs: introduce long xattr name prefixes feature Chao Yu
  7 siblings, 3 replies; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 14:17 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
 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


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

* Re: [PATCH 7/7] erofs: enable long extended attribute name prefixes
  2023-04-07 14:17 ` [PATCH 7/7] erofs: enable long extended attribute name prefixes Jingbo Xu
@ 2023-04-07 17:29   ` kernel test robot
  2023-04-07 18:22   ` kernel test robot
  2023-04-07 22:28   ` [PATCH v2 " Jingbo Xu
  2 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2023-04-07 17:29 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel, oe-kbuild-all

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/202304080101.D8cyKOoF-lkp@intel.com/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 <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304080101.D8cyKOoF-lkp@intel.com/

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

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

* Re: [PATCH 7/7] erofs: enable long extended attribute name prefixes
  2023-04-07 14:17 ` [PATCH 7/7] erofs: enable long extended attribute name prefixes Jingbo Xu
  2023-04-07 17:29   ` kernel test robot
@ 2023-04-07 18:22   ` kernel test robot
  2023-04-07 22:28   ` [PATCH v2 " Jingbo Xu
  2 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2023-04-07 18:22 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs
  Cc: llvm, linux-kernel, oe-kbuild-all

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/202304080206.t45iYSop-lkp@intel.com/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 <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304080206.t45iYSop-lkp@intel.com/

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

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

* [PATCH v2 7/7] erofs: enable long extended attribute name prefixes
  2023-04-07 14:17 ` [PATCH 7/7] erofs: enable long extended attribute name prefixes Jingbo Xu
  2023-04-07 17:29   ` kernel test robot
  2023-04-07 18:22   ` kernel test robot
@ 2023-04-07 22:28   ` Jingbo Xu
  2023-04-10  5:54     ` Gao Xiang
  2 siblings, 1 reply; 24+ messages in thread
From: Jingbo Xu @ 2023-04-07 22:28 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
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


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

* Re: [PATCH 2/7] erofs: initialize packed inode after root inode is assigned
  2023-04-07 14:17 ` [PATCH 2/7] erofs: initialize packed inode after root inode is assigned Jingbo Xu
@ 2023-04-09 10:52   ` Gao Xiang
  2023-04-10  2:44   ` Yue Hu
  1 sibling, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-09 10:52 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel



On 2023/4/7 22:17, Jingbo Xu wrote:
> As commit 8f7acdae2cd4 ("staging: erofs: kill all failure handling in
> fill_super()"), move the initialization of packed inode after root
> inode is assigned, so that the iput() in .put_super() is adequate as
> the failure handling.
> 
> Otherwise, iput() is also needed in .kill_sb(), in case of the mounting
> fails halfway.
> 

Fixes: b15b2e307c3a ("erofs: support on-disk compressed fragments data")

> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

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

* Re: [PATCH 3/7] erofs: move packed inode out of the compression part
  2023-04-07 14:17 ` [PATCH 3/7] erofs: move packed inode out of the compression part Jingbo Xu
@ 2023-04-09 10:53   ` Gao Xiang
  2023-04-10  2:45   ` Yue Hu
  1 sibling, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-09 10:53 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel



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 <jefflexu@linux.alibaba.com>

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

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

* Re: [PATCH 2/7] erofs: initialize packed inode after root inode is assigned
  2023-04-07 14:17 ` [PATCH 2/7] erofs: initialize packed inode after root inode is assigned Jingbo Xu
  2023-04-09 10:52   ` Gao Xiang
@ 2023-04-10  2:44   ` Yue Hu
  1 sibling, 0 replies; 24+ messages in thread
From: Yue Hu @ 2023-04-10  2:44 UTC (permalink / raw)
  To: Jingbo Xu; +Cc: linux-erofs, linux-kernel

On Fri,  7 Apr 2023 22:17:05 +0800
Jingbo Xu <jefflexu@linux.alibaba.com> wrote:

> As commit 8f7acdae2cd4 ("staging: erofs: kill all failure handling in
> fill_super()"), move the initialization of packed inode after root
> inode is assigned, so that the iput() in .put_super() is adequate as
> the failure handling.
> 
> Otherwise, iput() is also needed in .kill_sb(), in case of the mounting
> fails halfway.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

Reviewed-by: Yue Hu <huyue2@coolpad.com>

> ---
>  fs/erofs/internal.h |  1 +
>  fs/erofs/super.c    | 22 +++++++++++-----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 2bcff3194e4a..caea9dc1cd82 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -157,6 +157,7 @@ struct erofs_sb_info {
>  
>  	/* what we really care is nid, rather than ino.. */
>  	erofs_nid_t root_nid;
> +	erofs_nid_t packed_nid;
>  	/* used for statfs, f_files - f_favail */
>  	u64 inos;
>  
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 58ffbf410bfb..325602820dc8 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -388,17 +388,7 @@ static int erofs_read_superblock(struct super_block *sb)
>  #endif
>  	sbi->islotbits = ilog2(sizeof(struct erofs_inode_compact));
>  	sbi->root_nid = le16_to_cpu(dsb->root_nid);
> -#ifdef CONFIG_EROFS_FS_ZIP
> -	sbi->packed_inode = NULL;
> -	if (erofs_sb_has_fragments(sbi) && dsb->packed_nid) {
> -		sbi->packed_inode =
> -			erofs_iget(sb, le64_to_cpu(dsb->packed_nid));
> -		if (IS_ERR(sbi->packed_inode)) {
> -			ret = PTR_ERR(sbi->packed_inode);
> -			goto out;
> -		}
> -	}
> -#endif
> +	sbi->packed_nid = le64_to_cpu(dsb->packed_nid);
>  	sbi->inos = le64_to_cpu(dsb->inos);
>  
>  	sbi->build_time = le64_to_cpu(dsb->build_time);
> @@ -820,6 +810,16 @@ 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)) {
> +			err = PTR_ERR(sbi->packed_inode);
> +			sbi->packed_inode = NULL;
> +			return err;
> +		}
> +	}
> +#endif
>  	err = erofs_init_managed_cache(sb);
>  	if (err)
>  		return err;


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

* Re: [PATCH 3/7] erofs: move packed inode out of the compression part
  2023-04-07 14:17 ` [PATCH 3/7] erofs: move packed inode out of the compression part Jingbo Xu
  2023-04-09 10:53   ` Gao Xiang
@ 2023-04-10  2:45   ` Yue Hu
  1 sibling, 0 replies; 24+ messages in thread
From: Yue Hu @ 2023-04-10  2:45 UTC (permalink / raw)
  To: Jingbo Xu; +Cc: linux-kernel, zhangwen, huyue2, linux-erofs

On Fri,  7 Apr 2023 22:17:06 +0800
Jingbo Xu <jefflexu@linux.alibaba.com> 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 <jefflexu@linux.alibaba.com>

Reviewed-by: Yue Hu <huyue2@coolpad.com>

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


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

* Re: [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes
  2023-04-07 14:17 ` [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes Jingbo Xu
@ 2023-04-10  5:24   ` Gao Xiang
  0 siblings, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-10  5:24 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel



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 <jefflexu@linux.alibaba.com>

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

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)

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

* Re: [PATCH 5/7] erofs: add helpers to load long xattr name prefixes
  2023-04-07 14:17 ` [PATCH 5/7] erofs: add helpers to load " Jingbo Xu
@ 2023-04-10  5:44   ` Gao Xiang
  0 siblings, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-10  5:44 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel



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 <jefflexu@linux.alibaba.com>

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

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

* Re: [PATCH 6/7] erofs: handle long xattr name prefixes properly
  2023-04-07 14:17 ` [PATCH 6/7] erofs: handle long xattr name prefixes properly Jingbo Xu
@ 2023-04-10  5:53   ` Gao Xiang
  2023-04-10  6:39   ` [PATCH v2 " Jingbo Xu
  1 sibling, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-10  5:53 UTC (permalink / raw)
  To: Jingbo Xu; +Cc: linux-erofs mailing list, linux-kernel, Yue Hu



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 <jefflexu@linux.alibaba.com>
> ---
>   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 <hsiangkao@linux.alibaba.com>

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

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

* Re: [PATCH v2 7/7] erofs: enable long extended attribute name prefixes
  2023-04-07 22:28   ` [PATCH v2 " Jingbo Xu
@ 2023-04-10  5:54     ` Gao Xiang
  0 siblings, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-10  5:54 UTC (permalink / raw)
  To: Jingbo Xu; +Cc: linux-erofs mailing list, linux-kernel, Yue Hu



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 <jefflexu@linux.alibaba.com>
> ---
> v2: fix build error when CONFIG_EROFS_FS_XATTR is not defined

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

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

* [PATCH v2 6/7] erofs: handle long xattr name prefixes properly
  2023-04-07 14:17 ` [PATCH 6/7] erofs: handle long xattr name prefixes properly Jingbo Xu
  2023-04-10  5:53   ` Gao Xiang
@ 2023-04-10  6:39   ` Jingbo Xu
  2023-04-10  6:47     ` Gao Xiang
  2023-04-11  9:35     ` [PATCH v3 " Jingbo Xu
  1 sibling, 2 replies; 24+ messages in thread
From: Jingbo Xu @ 2023-04-10  6:39 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
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


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

* Re: [PATCH v2 6/7] erofs: handle long xattr name prefixes properly
  2023-04-10  6:39   ` [PATCH v2 " Jingbo Xu
@ 2023-04-10  6:47     ` Gao Xiang
  2023-04-11  9:35     ` [PATCH v3 " Jingbo Xu
  1 sibling, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-10  6:47 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel



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 <jefflexu@linux.alibaba.com>
> ---
> v2: remove "idx" temporary variable (Gao Xiang)

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

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

* [PATCH v3 6/7] erofs: handle long xattr name prefixes properly
  2023-04-10  6:39   ` [PATCH v2 " Jingbo Xu
  2023-04-10  6:47     ` Gao Xiang
@ 2023-04-11  9:35     ` Jingbo Xu
  2023-04-11  9:40       ` Gao Xiang
  1 sibling, 1 reply; 24+ messages in thread
From: Jingbo Xu @ 2023-04-11  9:35 UTC (permalink / raw)
  To: xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel

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 <jefflexu@linux.alibaba.com>
---
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


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

* Re: [PATCH v3 6/7] erofs: handle long xattr name prefixes properly
  2023-04-11  9:35     ` [PATCH v3 " Jingbo Xu
@ 2023-04-11  9:40       ` Gao Xiang
  0 siblings, 0 replies; 24+ messages in thread
From: Gao Xiang @ 2023-04-11  9:40 UTC (permalink / raw)
  To: Jingbo Xu, xiang, chao, huyue2, linux-erofs; +Cc: linux-kernel



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 <jefflexu@linux.alibaba.com>
> ---
> 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 <hsiangkao@linux.alibaba.com>

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

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

* Re: [PATCH 0/7] erofs: introduce long xattr name prefixes feature
  2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
                   ` (6 preceding siblings ...)
  2023-04-07 14:17 ` [PATCH 7/7] erofs: enable long extended attribute name prefixes Jingbo Xu
@ 2023-04-16 14:24 ` Chao Yu
  7 siblings, 0 replies; 24+ messages in thread
From: Chao Yu @ 2023-04-16 14:24 UTC (permalink / raw)
  To: Jingbo Xu, xiang, huyue2, linux-erofs; +Cc: linux-kernel

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 <chao@kernel.org>

Thanks,

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

end of thread, other threads:[~2023-04-16 14:24 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-07 14:17 [PATCH 0/7] erofs: introduce long xattr name prefixes feature Jingbo Xu
2023-04-07 14:17 ` [PATCH 1/7] erofs: keep meta inode into erofs_buf Jingbo Xu
2023-04-07 14:17 ` [PATCH 2/7] erofs: initialize packed inode after root inode is assigned Jingbo Xu
2023-04-09 10:52   ` Gao Xiang
2023-04-10  2:44   ` Yue Hu
2023-04-07 14:17 ` [PATCH 3/7] erofs: move packed inode out of the compression part Jingbo Xu
2023-04-09 10:53   ` Gao Xiang
2023-04-10  2:45   ` Yue Hu
2023-04-07 14:17 ` [PATCH 4/7] erofs: introduce on-disk format for long xattr name prefixes Jingbo Xu
2023-04-10  5:24   ` Gao Xiang
2023-04-07 14:17 ` [PATCH 5/7] erofs: add helpers to load " Jingbo Xu
2023-04-10  5:44   ` Gao Xiang
2023-04-07 14:17 ` [PATCH 6/7] erofs: handle long xattr name prefixes properly Jingbo Xu
2023-04-10  5:53   ` Gao Xiang
2023-04-10  6:39   ` [PATCH v2 " Jingbo Xu
2023-04-10  6:47     ` Gao Xiang
2023-04-11  9:35     ` [PATCH v3 " Jingbo Xu
2023-04-11  9:40       ` Gao Xiang
2023-04-07 14:17 ` [PATCH 7/7] erofs: enable long extended attribute name prefixes Jingbo Xu
2023-04-07 17:29   ` kernel test robot
2023-04-07 18:22   ` kernel test robot
2023-04-07 22:28   ` [PATCH v2 " Jingbo Xu
2023-04-10  5:54     ` Gao Xiang
2023-04-16 14:24 ` [PATCH 0/7] erofs: introduce long xattr name prefixes feature Chao Yu

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