[PATCH v2 0/8] erofs: cleanup of xattr handling

[PATCH v2 0/8] erofs: cleanup of xattr handling

[Jingbo Xu]

changes since v1:
- patch 1: keep erofs_xattr_handler() and erofs_getxattr() untouched in
  xattr.h (Gao Xiang)
- patch 2/3: add Reviewed-by tags (Gao Xiang)
- this refactoring can be tested by [1]

[1] https://lore.kernel.org/all/20230327123926.92934-1-jefflexu@linux.alibaba.com/

v1: https://lore.kernel.org/all/20230323000949.57608-1-jefflexu@linux.alibaba.com/

Jingbo Xu (8):
  erofs: move several xattr helpers into xattr.c
  erofs: rename init_inode_xattrs with erofs_ prefix
  erofs: simplify erofs_xattr_generic_get()
  erofs: introduce erofs_xattr_iter_fixup_aligned() helper
  erofs: unify xattr_iter structures
  erofs: make the size of read data stored in buffer_ofs
  erofs: unify inline/share xattr iterators for listxattr/getxattr
  erofs: use separate xattr parsers for listxattr/getxattr

 fs/erofs/xattr.c | 683 +++++++++++++++++++----------------------------
 fs/erofs/xattr.h |  23 --
 2 files changed, 281 insertions(+), 425 deletions(-)

-- 
2.19.1.6.gb485710b

[PATCH v2 1/8] erofs: move several xattr helpers into xattr.c

[Jingbo Xu]

Move xattrblock_addr() and xattrblock_offset() helpers into xattr.c,
as they are not used outside of xattr.c.

inlinexattr_header_size() has only one caller, and thus make it inlined
into the caller directly.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
 fs/erofs/xattr.c | 48 +++++++++++++++++++++++++++++-------------------
 fs/erofs/xattr.h | 23 -----------------------
 2 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 459caa3cd65d..9ccd57581bc7 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -7,6 +7,19 @@
 #include <linux/security.h>
 #include "xattr.h"
 
+static inline erofs_blk_t erofs_xattr_blkaddr(struct super_block *sb,
+					      unsigned int xattr_id)
+{
+	return EROFS_SB(sb)->xattr_blkaddr +
+	       erofs_blknr(sb, xattr_id * sizeof(__u32));
+}
+
+static inline unsigned int erofs_xattr_blkoff(struct super_block *sb,
+					      unsigned int xattr_id)
+{
+	return erofs_blkoff(sb, xattr_id * sizeof(__u32));
+}
+
 struct xattr_iter {
 	struct super_block *sb;
 	struct erofs_buf buf;
@@ -157,7 +170,8 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
 	struct erofs_inode *const vi = EROFS_I(inode);
 	unsigned int xattr_header_sz, inline_xattr_ofs;
 
-	xattr_header_sz = inlinexattr_header_size(inode);
+	xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
+			  sizeof(u32) * vi->xattr_shared_count;
 	if (xattr_header_sz >= vi->xattr_isize) {
 		DBG_BUGON(xattr_header_sz > vi->xattr_isize);
 		return -ENOATTR;
@@ -351,20 +365,18 @@ static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
 static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
 {
 	struct erofs_inode *const vi = EROFS_I(inode);
-	struct super_block *const sb = inode->i_sb;
-	unsigned int i;
+	struct super_block *const sb = it->it.sb;
+	unsigned int i, xsid;
 	int ret = -ENOATTR;
 
 	for (i = 0; i < vi->xattr_shared_count; ++i) {
-		erofs_blk_t blkaddr =
-			xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
-
-		it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
-		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
-						  EROFS_KMAP);
+		xsid = vi->xattr_shared_xattrs[i];
+		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
+		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
+		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb,
+						  it->it.blkaddr, EROFS_KMAP);
 		if (IS_ERR(it->it.kaddr))
 			return PTR_ERR(it->it.kaddr);
-		it->it.blkaddr = blkaddr;
 
 		ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
 		if (ret != -ENOATTR)
@@ -562,20 +574,18 @@ static int shared_listxattr(struct listxattr_iter *it)
 {
 	struct inode *const inode = d_inode(it->dentry);
 	struct erofs_inode *const vi = EROFS_I(inode);
-	struct super_block *const sb = inode->i_sb;
-	unsigned int i;
+	struct super_block *const sb = it->it.sb;
+	unsigned int i, xsid;
 	int ret = 0;
 
 	for (i = 0; i < vi->xattr_shared_count; ++i) {
-		erofs_blk_t blkaddr =
-			xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
-
-		it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
-		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
-						  EROFS_KMAP);
+		xsid = vi->xattr_shared_xattrs[i];
+		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
+		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
+		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb,
+						  it->it.blkaddr, EROFS_KMAP);
 		if (IS_ERR(it->it.kaddr))
 			return PTR_ERR(it->it.kaddr);
-		it->it.blkaddr = blkaddr;
 
 		ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
 		if (ret)
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index f7a21aaa9755..a65158cba14f 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -13,29 +13,6 @@
 /* Attribute not found */
 #define ENOATTR         ENODATA
 
-static inline unsigned int inlinexattr_header_size(struct inode *inode)
-{
-	return sizeof(struct erofs_xattr_ibody_header) +
-		sizeof(u32) * EROFS_I(inode)->xattr_shared_count;
-}
-
-static inline erofs_blk_t xattrblock_addr(struct super_block *sb,
-					  unsigned int xattr_id)
-{
-#ifdef CONFIG_EROFS_FS_XATTR
-	return EROFS_SB(sb)->xattr_blkaddr +
-		xattr_id * sizeof(__u32) / sb->s_blocksize;
-#else
-	return 0;
-#endif
-}
-
-static inline unsigned int xattrblock_offset(struct super_block *sb,
-					     unsigned int xattr_id)
-{
-	return (xattr_id * sizeof(__u32)) % sb->s_blocksize;
-}
-
 #ifdef CONFIG_EROFS_FS_XATTR
 extern const struct xattr_handler erofs_xattr_user_handler;
 extern const struct xattr_handler erofs_xattr_trusted_handler;
-- 
2.19.1.6.gb485710b

[PATCH v2 2/8] erofs: rename init_inode_xattrs with erofs_ prefix

[Jingbo Xu]

Rename init_inode_xattrs() to erofs_init_inode_xattrs() without logic
change.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/xattr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 9ccd57581bc7..dc36a0c0919c 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -29,7 +29,7 @@ struct xattr_iter {
 	unsigned int ofs;
 };
 
-static int init_inode_xattrs(struct inode *inode)
+static int erofs_init_inode_xattrs(struct inode *inode)
 {
 	struct erofs_inode *const vi = EROFS_I(inode);
 	struct xattr_iter it;
@@ -405,7 +405,7 @@ int erofs_getxattr(struct inode *inode, int index,
 	if (!name)
 		return -EINVAL;
 
-	ret = init_inode_xattrs(inode);
+	ret = erofs_init_inode_xattrs(inode);
 	if (ret)
 		return ret;
 
@@ -600,7 +600,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
 	int ret;
 	struct listxattr_iter it;
 
-	ret = init_inode_xattrs(d_inode(dentry));
+	ret = erofs_init_inode_xattrs(d_inode(dentry));
 	if (ret == -ENOATTR)
 		return 0;
 	if (ret)
-- 
2.19.1.6.gb485710b

[PATCH v2 3/8] erofs: simplify erofs_xattr_generic_get()

[Jingbo Xu]

erofs_xattr_generic_get() won't be called from xattr handlers other than
user/trusted/security xattr handler, and thus there's no need of extra
checking.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/xattr.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index dc36a0c0919c..d76b74ece2e5 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -432,20 +432,9 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
 				   struct dentry *unused, struct inode *inode,
 				   const char *name, void *buffer, size_t size)
 {
-	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
-
-	switch (handler->flags) {
-	case EROFS_XATTR_INDEX_USER:
-		if (!test_opt(&sbi->opt, XATTR_USER))
-			return -EOPNOTSUPP;
-		break;
-	case EROFS_XATTR_INDEX_TRUSTED:
-		break;
-	case EROFS_XATTR_INDEX_SECURITY:
-		break;
-	default:
-		return -EINVAL;
-	}
+	if (handler->flags == EROFS_XATTR_INDEX_USER &&
+	    !test_opt(&EROFS_I_SB(inode)->opt, XATTR_USER))
+		return -EOPNOTSUPP;
 
 	return erofs_getxattr(inode, handler->flags, name, buffer, size);
 }
-- 
2.19.1.6.gb485710b

[PATCH v2 4/8] erofs: introduce erofs_xattr_iter_fixup_aligned() helper

[Jingbo Xu]

Introduce erofs_xattr_iter_fixup_aligned() helper where
it.ofs <= EROFS_BLKSIZ is mandatory.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
 fs/erofs/xattr.c | 76 ++++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index d76b74ece2e5..f77d938613cc 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -29,6 +29,25 @@ struct xattr_iter {
 	unsigned int ofs;
 };
 
+static inline int erofs_xattr_iter_fixup(struct xattr_iter *it)
+{
+	if (it->ofs < it->sb->s_blocksize)
+		return 0;
+
+	it->blkaddr += erofs_blknr(it->sb, it->ofs);
+	it->kaddr = erofs_read_metabuf(&it->buf, it->sb, it->blkaddr, EROFS_KMAP);
+	if (IS_ERR(it->kaddr))
+		return PTR_ERR(it->kaddr);
+	it->ofs = erofs_blkoff(it->sb, it->ofs);
+	return 0;
+}
+
+static inline int erofs_xattr_iter_fixup_aligned(struct xattr_iter *it)
+{
+	DBG_BUGON(it->ofs > it->sb->s_blocksize);
+	return erofs_xattr_iter_fixup(it);
+}
+
 static int erofs_init_inode_xattrs(struct inode *inode)
 {
 	struct erofs_inode *const vi = EROFS_I(inode);
@@ -80,6 +99,7 @@ static int erofs_init_inode_xattrs(struct inode *inode)
 		goto out_unlock;
 	}
 
+	it.sb = sb;
 	it.buf = __EROFS_BUF_INITIALIZER;
 	it.blkaddr = erofs_blknr(sb, erofs_iloc(inode) + vi->inode_isize);
 	it.ofs = erofs_blkoff(sb, erofs_iloc(inode) + vi->inode_isize);
@@ -105,19 +125,11 @@ static int erofs_init_inode_xattrs(struct inode *inode)
 	it.ofs += sizeof(struct erofs_xattr_ibody_header);
 
 	for (i = 0; i < vi->xattr_shared_count; ++i) {
-		if (it.ofs >= sb->s_blocksize) {
-			/* cannot be unaligned */
-			DBG_BUGON(it.ofs != sb->s_blocksize);
-
-			it.kaddr = erofs_read_metabuf(&it.buf, sb, ++it.blkaddr,
-						      EROFS_KMAP);
-			if (IS_ERR(it.kaddr)) {
-				kfree(vi->xattr_shared_xattrs);
-				vi->xattr_shared_xattrs = NULL;
-				ret = PTR_ERR(it.kaddr);
-				goto out_unlock;
-			}
-			it.ofs = 0;
+		ret = erofs_xattr_iter_fixup_aligned(&it);
+		if (ret) {
+			kfree(vi->xattr_shared_xattrs);
+			vi->xattr_shared_xattrs = NULL;
+			goto out_unlock;
 		}
 		vi->xattr_shared_xattrs[i] =
 			le32_to_cpu(*(__le32 *)(it.kaddr + it.ofs));
@@ -150,20 +162,6 @@ struct xattr_iter_handlers {
 		      unsigned int len);
 };
 
-static inline int xattr_iter_fixup(struct xattr_iter *it)
-{
-	if (it->ofs < it->sb->s_blocksize)
-		return 0;
-
-	it->blkaddr += erofs_blknr(it->sb, it->ofs);
-	it->kaddr = erofs_read_metabuf(&it->buf, it->sb, it->blkaddr,
-				       EROFS_KMAP);
-	if (IS_ERR(it->kaddr))
-		return PTR_ERR(it->kaddr);
-	it->ofs = erofs_blkoff(it->sb, it->ofs);
-	return 0;
-}
-
 static int inline_xattr_iter_begin(struct xattr_iter *it,
 				   struct inode *inode)
 {
@@ -201,7 +199,7 @@ static int xattr_foreach(struct xattr_iter *it,
 	int err;
 
 	/* 0. fixup blkaddr, ofs, ipage */
-	err = xattr_iter_fixup(it);
+	err = erofs_xattr_iter_fixup(it);
 	if (err)
 		return err;
 
@@ -236,14 +234,9 @@ static int xattr_foreach(struct xattr_iter *it,
 	processed = 0;
 
 	while (processed < entry.e_name_len) {
-		if (it->ofs >= it->sb->s_blocksize) {
-			DBG_BUGON(it->ofs > it->sb->s_blocksize);
-
-			err = xattr_iter_fixup(it);
-			if (err)
-				goto out;
-			it->ofs = 0;
-		}
+		err = erofs_xattr_iter_fixup_aligned(it);
+		if (err)
+			goto out;
 
 		slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
 			      entry.e_name_len - processed);
@@ -271,14 +264,9 @@ static int xattr_foreach(struct xattr_iter *it,
 	}
 
 	while (processed < value_sz) {
-		if (it->ofs >= it->sb->s_blocksize) {
-			DBG_BUGON(it->ofs > it->sb->s_blocksize);
-
-			err = xattr_iter_fixup(it);
-			if (err)
-				goto out;
-			it->ofs = 0;
-		}
+		err = erofs_xattr_iter_fixup_aligned(it);
+		if (err)
+			goto out;
 
 		slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
 			      value_sz - processed);
-- 
2.19.1.6.gb485710b

[PATCH v2 5/8] erofs: unify xattr_iter structures

[Jingbo Xu]

Unify xattr_iter/listxattr_iter/getxattr_iter structures into
erofs_xattr_iter structure.

This is in preparation for the following further cleanup.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
 fs/erofs/xattr.c | 166 ++++++++++++++++++++---------------------------
 1 file changed, 69 insertions(+), 97 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index f77d938613cc..c9a893f84250 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -20,16 +20,22 @@ static inline unsigned int erofs_xattr_blkoff(struct super_block *sb,
 	return erofs_blkoff(sb, xattr_id * sizeof(__u32));
 }
 
-struct xattr_iter {
+struct erofs_xattr_iter {
 	struct super_block *sb;
 	struct erofs_buf buf;
 	void *kaddr;
-
 	erofs_blk_t blkaddr;
 	unsigned int ofs;
+
+	char *buffer;
+	int buffer_size, buffer_ofs;
+
+	int index;
+	struct qstr name;
+	struct dentry *dentry;
 };
 
-static inline int erofs_xattr_iter_fixup(struct xattr_iter *it)
+static inline int erofs_xattr_iter_fixup(struct erofs_xattr_iter *it)
 {
 	if (it->ofs < it->sb->s_blocksize)
 		return 0;
@@ -42,7 +48,7 @@ static inline int erofs_xattr_iter_fixup(struct xattr_iter *it)
 	return 0;
 }
 
-static inline int erofs_xattr_iter_fixup_aligned(struct xattr_iter *it)
+static inline int erofs_xattr_iter_fixup_aligned(struct erofs_xattr_iter *it)
 {
 	DBG_BUGON(it->ofs > it->sb->s_blocksize);
 	return erofs_xattr_iter_fixup(it);
@@ -51,7 +57,7 @@ static inline int erofs_xattr_iter_fixup_aligned(struct xattr_iter *it)
 static int erofs_init_inode_xattrs(struct inode *inode)
 {
 	struct erofs_inode *const vi = EROFS_I(inode);
-	struct xattr_iter it;
+	struct erofs_xattr_iter it;
 	unsigned int i;
 	struct erofs_xattr_ibody_header *ih;
 	struct super_block *sb = inode->i_sb;
@@ -154,15 +160,15 @@ static int erofs_init_inode_xattrs(struct inode *inode)
  *                            and need to be handled
  */
 struct xattr_iter_handlers {
-	int (*entry)(struct xattr_iter *_it, struct erofs_xattr_entry *entry);
-	int (*name)(struct xattr_iter *_it, unsigned int processed, char *buf,
+	int (*entry)(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry);
+	int (*name)(struct erofs_xattr_iter *it, unsigned int processed, char *buf,
 		    unsigned int len);
-	int (*alloc_buffer)(struct xattr_iter *_it, unsigned int value_sz);
-	void (*value)(struct xattr_iter *_it, unsigned int processed, char *buf,
+	int (*alloc_buffer)(struct erofs_xattr_iter *it, unsigned int value_sz);
+	void (*value)(struct erofs_xattr_iter *it, unsigned int processed, char *buf,
 		      unsigned int len);
 };
 
-static int inline_xattr_iter_begin(struct xattr_iter *it,
+static int inline_xattr_iter_begin(struct erofs_xattr_iter *it,
 				   struct inode *inode)
 {
 	struct erofs_inode *const vi = EROFS_I(inode);
@@ -190,7 +196,7 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
  * Regardless of success or failure, `xattr_foreach' will end up with
  * `ofs' pointing to the next xattr item rather than an arbitrary position.
  */
-static int xattr_foreach(struct xattr_iter *it,
+static int xattr_foreach(struct erofs_xattr_iter *it,
 			 const struct xattr_iter_handlers *op,
 			 unsigned int *tlimit)
 {
@@ -281,47 +287,32 @@ static int xattr_foreach(struct xattr_iter *it,
 	return err < 0 ? err : 0;
 }
 
-struct getxattr_iter {
-	struct xattr_iter it;
-
-	char *buffer;
-	int buffer_size, index;
-	struct qstr name;
-};
-
-static int xattr_entrymatch(struct xattr_iter *_it,
+static int xattr_entrymatch(struct erofs_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;
 }
 
-static int xattr_namematch(struct xattr_iter *_it,
+static int xattr_namematch(struct erofs_xattr_iter *it,
 			   unsigned int processed, char *buf, unsigned int len)
 {
-	struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
-
 	return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 0;
 }
 
-static int xattr_checkbuffer(struct xattr_iter *_it,
+static int xattr_checkbuffer(struct erofs_xattr_iter *it,
 			     unsigned int value_sz)
 {
-	struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
 	int err = it->buffer_size < value_sz ? -ERANGE : 0;
 
 	it->buffer_size = value_sz;
 	return !it->buffer ? 1 : err;
 }
 
-static void xattr_copyvalue(struct xattr_iter *_it,
+static void xattr_copyvalue(struct erofs_xattr_iter *it,
 			    unsigned int processed,
 			    char *buf, unsigned int len)
 {
-	struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
-
 	memcpy(it->buffer + processed, buf, len);
 }
 
@@ -332,41 +323,40 @@ static const struct xattr_iter_handlers find_xattr_handlers = {
 	.value = xattr_copyvalue
 };
 
-static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
+static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
 {
 	int ret;
 	unsigned int remaining;
 
-	ret = inline_xattr_iter_begin(&it->it, inode);
+	ret = inline_xattr_iter_begin(it, inode);
 	if (ret < 0)
 		return ret;
 
 	remaining = ret;
 	while (remaining) {
-		ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
+		ret = xattr_foreach(it, &find_xattr_handlers, &remaining);
 		if (ret != -ENOATTR)
 			break;
 	}
 	return ret ? ret : it->buffer_size;
 }
 
-static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
+static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
 {
 	struct erofs_inode *const vi = EROFS_I(inode);
-	struct super_block *const sb = it->it.sb;
+	struct super_block *const sb = it->sb;
 	unsigned int i, xsid;
 	int ret = -ENOATTR;
 
 	for (i = 0; i < vi->xattr_shared_count; ++i) {
 		xsid = vi->xattr_shared_xattrs[i];
-		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
-		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
-		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb,
-						  it->it.blkaddr, EROFS_KMAP);
-		if (IS_ERR(it->it.kaddr))
-			return PTR_ERR(it->it.kaddr);
-
-		ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
+		it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
+		it->ofs = erofs_xattr_blkoff(sb, xsid);
+		it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, EROFS_KMAP);
+		if (IS_ERR(it->kaddr))
+			return PTR_ERR(it->kaddr);
+
+		ret = xattr_foreach(it, &find_xattr_handlers, NULL);
 		if (ret != -ENOATTR)
 			break;
 	}
@@ -388,31 +378,30 @@ int erofs_getxattr(struct inode *inode, int index,
 		   void *buffer, size_t buffer_size)
 {
 	int ret;
-	struct getxattr_iter it;
+	struct erofs_xattr_iter it;
 
 	if (!name)
 		return -EINVAL;
+	if (strlen(name) > EROFS_NAME_LEN)
+		return -ERANGE;
 
 	ret = erofs_init_inode_xattrs(inode);
 	if (ret)
 		return ret;
 
-	it.index = index;
-	it.name.len = strlen(name);
-	if (it.name.len > EROFS_NAME_LEN)
-		return -ERANGE;
-
-	it.it.buf = __EROFS_BUF_INITIALIZER;
-	it.name.name = name;
-
-	it.buffer = buffer;
-	it.buffer_size = buffer_size;
+	it = (struct erofs_xattr_iter) {
+		.buf	     = __EROFS_BUF_INITIALIZER,
+		.sb	     = inode->i_sb,
+		.name	     = QSTR_INIT(name, strlen(name)),
+		.index	     = index,
+		.buffer	     = buffer,
+		.buffer_size = buffer_size,
+	};
 
-	it.it.sb = inode->i_sb;
 	ret = inline_getxattr(inode, &it);
 	if (ret == -ENOATTR)
 		ret = shared_getxattr(inode, &it);
-	erofs_put_metabuf(&it.it.buf);
+	erofs_put_metabuf(&it.buf);
 	return ret;
 }
 
@@ -462,19 +451,9 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
 	NULL,
 };
 
-struct listxattr_iter {
-	struct xattr_iter it;
-
-	struct dentry *dentry;
-	char *buffer;
-	int buffer_size, buffer_ofs;
-};
-
-static int xattr_entrylist(struct xattr_iter *_it,
+static int xattr_entrylist(struct erofs_xattr_iter *it,
 			   struct erofs_xattr_entry *entry)
 {
-	struct listxattr_iter *it =
-		container_of(_it, struct listxattr_iter, it);
 	unsigned int prefix_len;
 	const char *prefix;
 
@@ -501,23 +480,17 @@ static int xattr_entrylist(struct xattr_iter *_it,
 	return 0;
 }
 
-static int xattr_namelist(struct xattr_iter *_it,
+static int xattr_namelist(struct erofs_xattr_iter *it,
 			  unsigned int processed, char *buf, unsigned int len)
 {
-	struct listxattr_iter *it =
-		container_of(_it, struct listxattr_iter, it);
-
 	memcpy(it->buffer + it->buffer_ofs, buf, len);
 	it->buffer_ofs += len;
 	return 0;
 }
 
-static int xattr_skipvalue(struct xattr_iter *_it,
+static int xattr_skipvalue(struct erofs_xattr_iter *it,
 			   unsigned int value_sz)
 {
-	struct listxattr_iter *it =
-		container_of(_it, struct listxattr_iter, it);
-
 	it->buffer[it->buffer_ofs++] = '\0';
 	return 1;
 }
@@ -529,42 +502,41 @@ static const struct xattr_iter_handlers list_xattr_handlers = {
 	.value = NULL
 };
 
-static int inline_listxattr(struct listxattr_iter *it)
+static int inline_listxattr(struct erofs_xattr_iter *it)
 {
 	int ret;
 	unsigned int remaining;
 
-	ret = inline_xattr_iter_begin(&it->it, d_inode(it->dentry));
+	ret = inline_xattr_iter_begin(it, d_inode(it->dentry));
 	if (ret < 0)
 		return ret;
 
 	remaining = ret;
 	while (remaining) {
-		ret = xattr_foreach(&it->it, &list_xattr_handlers, &remaining);
+		ret = xattr_foreach(it, &list_xattr_handlers, &remaining);
 		if (ret)
 			break;
 	}
 	return ret ? ret : it->buffer_ofs;
 }
 
-static int shared_listxattr(struct listxattr_iter *it)
+static int shared_listxattr(struct erofs_xattr_iter *it)
 {
 	struct inode *const inode = d_inode(it->dentry);
 	struct erofs_inode *const vi = EROFS_I(inode);
-	struct super_block *const sb = it->it.sb;
+	struct super_block *const sb = it->sb;
 	unsigned int i, xsid;
 	int ret = 0;
 
 	for (i = 0; i < vi->xattr_shared_count; ++i) {
 		xsid = vi->xattr_shared_xattrs[i];
-		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
-		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
-		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb,
-						  it->it.blkaddr, EROFS_KMAP);
-		if (IS_ERR(it->it.kaddr))
-			return PTR_ERR(it->it.kaddr);
-
-		ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
+		it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
+		it->ofs = erofs_xattr_blkoff(sb, xsid);
+		it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, EROFS_KMAP);
+		if (IS_ERR(it->kaddr))
+			return PTR_ERR(it->kaddr);
+
+		ret = xattr_foreach(it, &list_xattr_handlers, NULL);
 		if (ret)
 			break;
 	}
@@ -575,7 +547,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
 			char *buffer, size_t buffer_size)
 {
 	int ret;
-	struct listxattr_iter it;
+	struct erofs_xattr_iter it;
 
 	ret = erofs_init_inode_xattrs(d_inode(dentry));
 	if (ret == -ENOATTR)
@@ -583,18 +555,18 @@ ssize_t erofs_listxattr(struct dentry *dentry,
 	if (ret)
 		return ret;
 
-	it.it.buf = __EROFS_BUF_INITIALIZER;
-	it.dentry = dentry;
-	it.buffer = buffer;
-	it.buffer_size = buffer_size;
-	it.buffer_ofs = 0;
-
-	it.it.sb = dentry->d_sb;
+	it = (struct erofs_xattr_iter) {
+		.buf	     = __EROFS_BUF_INITIALIZER,
+		.sb	     = dentry->d_sb,
+		.dentry	     = dentry,
+		.buffer	     = buffer,
+		.buffer_size = buffer_size,
+	};
 
 	ret = inline_listxattr(&it);
 	if (ret >= 0 || ret == -ENOATTR)
 		ret = shared_listxattr(&it);
-	erofs_put_metabuf(&it.it.buf);
+	erofs_put_metabuf(&it.buf);
 	return ret;
 }
 
-- 
2.19.1.6.gb485710b

[PATCH v2 6/8] erofs: make the size of read data stored in buffer_ofs

[Jingbo Xu]

Since now xattr_iter structures have been unified, make the size of the
read data stored in buffer_ofs.  Don't bother reusing buffer_size for
this use, which may be confusing.

This is in preparation for the following further cleanup.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
 fs/erofs/xattr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index c9a893f84250..5bd2c2fcbafd 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -305,7 +305,7 @@ static int xattr_checkbuffer(struct erofs_xattr_iter *it,
 {
 	int err = it->buffer_size < value_sz ? -ERANGE : 0;
 
-	it->buffer_size = value_sz;
+	it->buffer_ofs = value_sz;
 	return !it->buffer ? 1 : err;
 }
 
@@ -338,7 +338,7 @@ static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
 		if (ret != -ENOATTR)
 			break;
 	}
-	return ret ? ret : it->buffer_size;
+	return ret ? ret : it->buffer_ofs;
 }
 
 static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
@@ -360,7 +360,7 @@ static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
 		if (ret != -ENOATTR)
 			break;
 	}
-	return ret ? ret : it->buffer_size;
+	return ret ? ret : it->buffer_ofs;
 }
 
 static bool erofs_xattr_user_list(struct dentry *dentry)
-- 
2.19.1.6.gb485710b

[PATCH v2 7/8] erofs: unify inline/share xattr iterators for listxattr/getxattr

[Jingbo Xu]

Make inline_getxattr() and inline_listxattr() unified as
iter_inline_xattr(), shared_getxattr() and shared_listxattr() unified as
iter_shared_xattr().

After the unification, both iter_inline_xattr() and iter_shared_xattr()
return 0 on success, and negative error on failure.

One thing worth noting is that, the logic of returning it->buffer_ofs
when there's no shared xattrs in shared_listxattr() is moved to
erofs_listxattr() to make the unification possible.  The only difference
is that, semantically the old behavior will return ENOATTR rather than
it->buffer_ofs if ENOATTR encountered when listxattr is parsing upon a
specific shared xattr, while now the new behavior will return
it->buffer_ofs in this case.  This is not an issue, as listxattr upon a
specific xattr won't return ENOATTR.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
 fs/erofs/xattr.c | 211 ++++++++++++++++++-----------------------------
 1 file changed, 80 insertions(+), 131 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 5bd2c2fcbafd..541d508e7758 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -7,19 +7,6 @@
 #include <linux/security.h>
 #include "xattr.h"
 
-static inline erofs_blk_t erofs_xattr_blkaddr(struct super_block *sb,
-					      unsigned int xattr_id)
-{
-	return EROFS_SB(sb)->xattr_blkaddr +
-	       erofs_blknr(sb, xattr_id * sizeof(__u32));
-}
-
-static inline unsigned int erofs_xattr_blkoff(struct super_block *sb,
-					      unsigned int xattr_id)
-{
-	return erofs_blkoff(sb, xattr_id * sizeof(__u32));
-}
-
 struct erofs_xattr_iter {
 	struct super_block *sb;
 	struct erofs_buf buf;
@@ -33,6 +20,8 @@ struct erofs_xattr_iter {
 	int index;
 	struct qstr name;
 	struct dentry *dentry;
+	struct inode *inode;
+	bool getxattr;
 };
 
 static inline int erofs_xattr_iter_fixup(struct erofs_xattr_iter *it)
@@ -168,30 +157,6 @@ struct xattr_iter_handlers {
 		      unsigned int len);
 };
 
-static int inline_xattr_iter_begin(struct erofs_xattr_iter *it,
-				   struct inode *inode)
-{
-	struct erofs_inode *const vi = EROFS_I(inode);
-	unsigned int xattr_header_sz, inline_xattr_ofs;
-
-	xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
-			  sizeof(u32) * vi->xattr_shared_count;
-	if (xattr_header_sz >= vi->xattr_isize) {
-		DBG_BUGON(xattr_header_sz > vi->xattr_isize);
-		return -ENOATTR;
-	}
-
-	inline_xattr_ofs = vi->inode_isize + xattr_header_sz;
-
-	it->blkaddr = erofs_blknr(it->sb, erofs_iloc(inode) + inline_xattr_ofs);
-	it->ofs = erofs_blkoff(it->sb, erofs_iloc(inode) + inline_xattr_ofs);
-	it->kaddr = erofs_read_metabuf(&it->buf, inode->i_sb, it->blkaddr,
-				       EROFS_KMAP);
-	if (IS_ERR(it->kaddr))
-		return PTR_ERR(it->kaddr);
-	return vi->xattr_isize - xattr_header_sz;
-}
-
 /*
  * Regardless of success or failure, `xattr_foreach' will end up with
  * `ofs' pointing to the next xattr item rather than an arbitrary position.
@@ -323,46 +288,6 @@ static const struct xattr_iter_handlers find_xattr_handlers = {
 	.value = xattr_copyvalue
 };
 
-static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
-{
-	int ret;
-	unsigned int remaining;
-
-	ret = inline_xattr_iter_begin(it, inode);
-	if (ret < 0)
-		return ret;
-
-	remaining = ret;
-	while (remaining) {
-		ret = xattr_foreach(it, &find_xattr_handlers, &remaining);
-		if (ret != -ENOATTR)
-			break;
-	}
-	return ret ? ret : it->buffer_ofs;
-}
-
-static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
-{
-	struct erofs_inode *const vi = EROFS_I(inode);
-	struct super_block *const sb = it->sb;
-	unsigned int i, xsid;
-	int ret = -ENOATTR;
-
-	for (i = 0; i < vi->xattr_shared_count; ++i) {
-		xsid = vi->xattr_shared_xattrs[i];
-		it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
-		it->ofs = erofs_xattr_blkoff(sb, xsid);
-		it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, EROFS_KMAP);
-		if (IS_ERR(it->kaddr))
-			return PTR_ERR(it->kaddr);
-
-		ret = xattr_foreach(it, &find_xattr_handlers, NULL);
-		if (ret != -ENOATTR)
-			break;
-	}
-	return ret ? ret : it->buffer_ofs;
-}
-
 static bool erofs_xattr_user_list(struct dentry *dentry)
 {
 	return test_opt(&EROFS_SB(dentry->d_sb)->opt, XATTR_USER);
@@ -373,38 +298,6 @@ static bool erofs_xattr_trusted_list(struct dentry *dentry)
 	return capable(CAP_SYS_ADMIN);
 }
 
-int erofs_getxattr(struct inode *inode, int index,
-		   const char *name,
-		   void *buffer, size_t buffer_size)
-{
-	int ret;
-	struct erofs_xattr_iter it;
-
-	if (!name)
-		return -EINVAL;
-	if (strlen(name) > EROFS_NAME_LEN)
-		return -ERANGE;
-
-	ret = erofs_init_inode_xattrs(inode);
-	if (ret)
-		return ret;
-
-	it = (struct erofs_xattr_iter) {
-		.buf	     = __EROFS_BUF_INITIALIZER,
-		.sb	     = inode->i_sb,
-		.name	     = QSTR_INIT(name, strlen(name)),
-		.index	     = index,
-		.buffer	     = buffer,
-		.buffer_size = buffer_size,
-	};
-
-	ret = inline_getxattr(inode, &it);
-	if (ret == -ENOATTR)
-		ret = shared_getxattr(inode, &it);
-	erofs_put_metabuf(&it.buf);
-	return ret;
-}
-
 static int erofs_xattr_generic_get(const struct xattr_handler *handler,
 				   struct dentry *unused, struct inode *inode,
 				   const char *name, void *buffer, size_t size)
@@ -502,45 +395,97 @@ static const struct xattr_iter_handlers list_xattr_handlers = {
 	.value = NULL
 };
 
-static int inline_listxattr(struct erofs_xattr_iter *it)
+static int erofs_iter_inline_xattr(struct erofs_xattr_iter *it)
 {
+	struct erofs_inode *const vi = EROFS_I(it->inode);
+	const struct xattr_iter_handlers *op;
+	unsigned int xattr_header_sz, remaining;
+	erofs_off_t pos;
 	int ret;
-	unsigned int remaining;
 
-	ret = inline_xattr_iter_begin(it, d_inode(it->dentry));
-	if (ret < 0)
-		return ret;
+	xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
+			  sizeof(u32) * vi->xattr_shared_count;
+	if (xattr_header_sz >= vi->xattr_isize) {
+		DBG_BUGON(xattr_header_sz > vi->xattr_isize);
+		return -ENOATTR;
+	}
+
+	pos = erofs_iloc(it->inode) + vi->inode_isize + xattr_header_sz;
+	it->blkaddr = erofs_blknr(it->sb, pos);
+	it->ofs = erofs_blkoff(it->sb, pos);
+	it->kaddr = erofs_read_metabuf(&it->buf, it->sb, it->blkaddr, EROFS_KMAP);
+	if (IS_ERR(it->kaddr))
+		return PTR_ERR(it->kaddr);
+
+	remaining = vi->xattr_isize - xattr_header_sz;
+	op = it->getxattr ? &find_xattr_handlers : &list_xattr_handlers;
 
-	remaining = ret;
 	while (remaining) {
-		ret = xattr_foreach(it, &list_xattr_handlers, &remaining);
-		if (ret)
+		ret = xattr_foreach(it, op, &remaining);
+		if ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
 			break;
 	}
-	return ret ? ret : it->buffer_ofs;
+	return ret;
 }
 
-static int shared_listxattr(struct erofs_xattr_iter *it)
+static int erofs_iter_shared_xattr(struct erofs_xattr_iter *it)
 {
-	struct inode *const inode = d_inode(it->dentry);
-	struct erofs_inode *const vi = EROFS_I(inode);
+	struct erofs_inode *const vi = EROFS_I(it->inode);
 	struct super_block *const sb = it->sb;
+	const struct xattr_iter_handlers *op;
 	unsigned int i, xsid;
-	int ret = 0;
+	int ret = -ENOATTR;
+
+	op = it->getxattr ? &find_xattr_handlers : &list_xattr_handlers;
 
 	for (i = 0; i < vi->xattr_shared_count; ++i) {
 		xsid = vi->xattr_shared_xattrs[i];
-		it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
-		it->ofs = erofs_xattr_blkoff(sb, xsid);
+		it->blkaddr = EROFS_SB(sb)->xattr_blkaddr +
+			      erofs_blknr(sb, xsid * sizeof(__u32));
+		it->ofs = erofs_blkoff(sb, xsid * sizeof(__u32));
 		it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, EROFS_KMAP);
 		if (IS_ERR(it->kaddr))
 			return PTR_ERR(it->kaddr);
 
-		ret = xattr_foreach(it, &list_xattr_handlers, NULL);
-		if (ret)
+		ret = xattr_foreach(it, op, NULL);
+		if ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
 			break;
 	}
-	return ret ? ret : it->buffer_ofs;
+	return ret;
+}
+
+int erofs_getxattr(struct inode *inode, int index,
+		   const char *name,
+		   void *buffer, size_t buffer_size)
+{
+	int ret;
+	struct erofs_xattr_iter it;
+
+	if (!name)
+		return -EINVAL;
+	if (strlen(name) > EROFS_NAME_LEN)
+		return -ERANGE;
+
+	ret = erofs_init_inode_xattrs(inode);
+	if (ret)
+		return ret;
+
+	it = (struct erofs_xattr_iter) {
+		.buf	     = __EROFS_BUF_INITIALIZER,
+		.sb	     = inode->i_sb,
+		.inode	     = inode,
+		.name	     = QSTR_INIT(name, strlen(name)),
+		.index	     = index,
+		.buffer	     = buffer,
+		.buffer_size = buffer_size,
+		.getxattr    = true,
+	};
+
+	ret = erofs_iter_inline_xattr(&it);
+	if (ret == -ENOATTR)
+		ret = erofs_iter_shared_xattr(&it);
+	erofs_put_metabuf(&it.buf);
+	return ret ? ret : it.buffer_ofs;
 }
 
 ssize_t erofs_listxattr(struct dentry *dentry,
@@ -559,15 +504,19 @@ ssize_t erofs_listxattr(struct dentry *dentry,
 		.buf	     = __EROFS_BUF_INITIALIZER,
 		.sb	     = dentry->d_sb,
 		.dentry	     = dentry,
+		.inode	     = d_inode(dentry),
 		.buffer	     = buffer,
 		.buffer_size = buffer_size,
+		.getxattr    = false,
 	};
 
-	ret = inline_listxattr(&it);
-	if (ret >= 0 || ret == -ENOATTR)
-		ret = shared_listxattr(&it);
+	ret = erofs_iter_inline_xattr(&it);
+	if (!ret || ret == -ENOATTR)
+		ret = erofs_iter_shared_xattr(&it);
+	if (ret == -ENOATTR)
+		ret = 0;
 	erofs_put_metabuf(&it.buf);
-	return ret;
+	return ret ? ret : it.buffer_ofs;
 }
 
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
-- 
2.19.1.6.gb485710b

[PATCH v2 8/8] erofs: use separate xattr parsers for listxattr/getxattr

[Jingbo Xu]

There's a callback styled xattr parser, i.e. xattr_foreach(), which is
shared among listxattr and getxattr.  Convert it to two separate xattr
parsers for listxattr and getxattr.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
 fs/erofs/xattr.c | 347 ++++++++++++++++++++++-------------------------
 1 file changed, 159 insertions(+), 188 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 541d508e7758..8851c38e8ca7 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -21,6 +21,7 @@ struct erofs_xattr_iter {
 	struct qstr name;
 	struct dentry *dentry;
 	struct inode *inode;
+	unsigned int remaining; /* size of inline xattrs to be iterated */
 	bool getxattr;
 };
 
@@ -141,153 +142,6 @@ static int erofs_init_inode_xattrs(struct inode *inode)
 	return ret;
 }
 
-/*
- * the general idea for these return values is
- * if    0 is returned, go on processing the current xattr;
- *       1 (> 0) is returned, skip this round to process the next xattr;
- *    -err (< 0) is returned, an error (maybe ENOXATTR) occurred
- *                            and need to be handled
- */
-struct xattr_iter_handlers {
-	int (*entry)(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry);
-	int (*name)(struct erofs_xattr_iter *it, unsigned int processed, char *buf,
-		    unsigned int len);
-	int (*alloc_buffer)(struct erofs_xattr_iter *it, unsigned int value_sz);
-	void (*value)(struct erofs_xattr_iter *it, unsigned int processed, char *buf,
-		      unsigned int len);
-};
-
-/*
- * Regardless of success or failure, `xattr_foreach' will end up with
- * `ofs' pointing to the next xattr item rather than an arbitrary position.
- */
-static int xattr_foreach(struct erofs_xattr_iter *it,
-			 const struct xattr_iter_handlers *op,
-			 unsigned int *tlimit)
-{
-	struct erofs_xattr_entry entry;
-	unsigned int value_sz, processed, slice;
-	int err;
-
-	/* 0. fixup blkaddr, ofs, ipage */
-	err = erofs_xattr_iter_fixup(it);
-	if (err)
-		return err;
-
-	/*
-	 * 1. read xattr entry to the memory,
-	 *    since we do EROFS_XATTR_ALIGN
-	 *    therefore entry should be in the page
-	 */
-	entry = *(struct erofs_xattr_entry *)(it->kaddr + it->ofs);
-	if (tlimit) {
-		unsigned int entry_sz = erofs_xattr_entry_size(&entry);
-
-		/* xattr on-disk corruption: xattr entry beyond xattr_isize */
-		if (*tlimit < entry_sz) {
-			DBG_BUGON(1);
-			return -EFSCORRUPTED;
-		}
-		*tlimit -= entry_sz;
-	}
-
-	it->ofs += sizeof(struct erofs_xattr_entry);
-	value_sz = le16_to_cpu(entry.e_value_size);
-
-	/* handle entry */
-	err = op->entry(it, &entry);
-	if (err) {
-		it->ofs += entry.e_name_len + value_sz;
-		goto out;
-	}
-
-	/* 2. handle xattr name (ofs will finally be at the end of name) */
-	processed = 0;
-
-	while (processed < entry.e_name_len) {
-		err = erofs_xattr_iter_fixup_aligned(it);
-		if (err)
-			goto out;
-
-		slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
-			      entry.e_name_len - processed);
-
-		/* handle name */
-		err = op->name(it, processed, it->kaddr + it->ofs, slice);
-		if (err) {
-			it->ofs += entry.e_name_len - processed + value_sz;
-			goto out;
-		}
-
-		it->ofs += slice;
-		processed += slice;
-	}
-
-	/* 3. handle xattr value */
-	processed = 0;
-
-	if (op->alloc_buffer) {
-		err = op->alloc_buffer(it, value_sz);
-		if (err) {
-			it->ofs += value_sz;
-			goto out;
-		}
-	}
-
-	while (processed < value_sz) {
-		err = erofs_xattr_iter_fixup_aligned(it);
-		if (err)
-			goto out;
-
-		slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
-			      value_sz - processed);
-		op->value(it, processed, it->kaddr + it->ofs, slice);
-		it->ofs += slice;
-		processed += slice;
-	}
-
-out:
-	/* xattrs should be 4-byte aligned (on-disk constraint) */
-	it->ofs = EROFS_XATTR_ALIGN(it->ofs);
-	return err < 0 ? err : 0;
-}
-
-static int xattr_entrymatch(struct erofs_xattr_iter *it,
-			    struct erofs_xattr_entry *entry)
-{
-	return (it->index != entry->e_name_index ||
-		it->name.len != entry->e_name_len) ? -ENOATTR : 0;
-}
-
-static int xattr_namematch(struct erofs_xattr_iter *it,
-			   unsigned int processed, char *buf, unsigned int len)
-{
-	return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 0;
-}
-
-static int xattr_checkbuffer(struct erofs_xattr_iter *it,
-			     unsigned int value_sz)
-{
-	int err = it->buffer_size < value_sz ? -ERANGE : 0;
-
-	it->buffer_ofs = value_sz;
-	return !it->buffer ? 1 : err;
-}
-
-static void xattr_copyvalue(struct erofs_xattr_iter *it,
-			    unsigned int processed,
-			    char *buf, unsigned int len)
-{
-	memcpy(it->buffer + processed, buf, len);
-}
-
-static const struct xattr_iter_handlers find_xattr_handlers = {
-	.entry = xattr_entrymatch,
-	.name = xattr_namematch,
-	.alloc_buffer = xattr_checkbuffer,
-	.value = xattr_copyvalue
-};
-
 static bool erofs_xattr_user_list(struct dentry *dentry)
 {
 	return test_opt(&EROFS_SB(dentry->d_sb)->opt, XATTR_USER);
@@ -344,62 +198,178 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
 	NULL,
 };
 
-static int xattr_entrylist(struct erofs_xattr_iter *it,
-			   struct erofs_xattr_entry *entry)
+/*
+ * the general idea for these return values is
+ * if    0 is returned, go on processing the current xattr;
+ *       1 (> 0) is returned, skip this round to process the next xattr;
+ *    -err (< 0) is returned, an error (maybe ENOXATTR) occurred
+ *                            and need to be handled
+ */
+typedef int (*erofs_xattr_body_handler)(struct erofs_xattr_iter *it,
+					unsigned int processed,
+					char *buf, unsigned int len);
+
+static int erofs_xattr_namematch(struct erofs_xattr_iter *it,
+		unsigned int processed, char *buf, unsigned int len)
+{
+	return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 0;
+}
+
+static int erofs_xattr_copy(struct erofs_xattr_iter *it,
+		unsigned int unused, char *buf, unsigned int len)
+{
+	memcpy(it->buffer + it->buffer_ofs, buf, len);
+	it->buffer_ofs += len;
+	return 0;
+}
+
+static int erofs_xattr_body(struct erofs_xattr_iter *it, unsigned int len,
+			    erofs_xattr_body_handler handler)
+{
+	unsigned int slice, processed = 0;
+
+	while (processed < len) {
+		int err = erofs_xattr_iter_fixup_aligned(it);
+		if (err)
+			return err;
+
+		slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
+			      len - processed);
+		err = handler(it, processed, it->kaddr + it->ofs, slice);
+		if (err) {
+			it->ofs += len - processed;
+			return err;
+		}
+
+		it->ofs += slice;
+		processed += slice;
+	}
+	return 0;
+}
+
+static int erofs_xattr_check_entry(struct erofs_xattr_iter *it)
 {
-	unsigned int prefix_len;
+	int err;
+
+	err = erofs_xattr_iter_fixup(it);
+	if (err)
+		return err;
+
+	if (it->remaining) {
+		struct erofs_xattr_entry *entry = it->kaddr + it->ofs;
+		unsigned int entry_sz = erofs_xattr_entry_size(entry);
+		/* xattr on-disk corruption: xattr entry beyond xattr_isize */
+		if (it->remaining < entry_sz) {
+			DBG_BUGON(1);
+			return -EFSCORRUPTED;
+		}
+		it->remaining -= entry_sz;
+	}
+	return 0;
+}
+
+/*
+ * Wen returning 0 or ENOATTR, erofs_[get|list]xattr_foreach() will end up
+ * with `ofs' pointing to the next xattr item rather than an arbitrary position.
+ */
+static int erofs_listxattr_foreach(struct erofs_xattr_iter *it)
+{
+	struct erofs_xattr_entry entry;
+	const struct xattr_handler *h;
 	const char *prefix;
+	unsigned int value_sz, prefix_len, name_sz;
+	int err;
 
-	const struct xattr_handler *h =
-		erofs_xattr_handler(entry->e_name_index);
+	err = erofs_xattr_check_entry(it);
+	if (err)
+		return err;
 
-	if (!h || (h->list && !h->list(it->dentry)))
-		return 1;
+	/* 1. handle xattr entry */
+	entry = *(struct erofs_xattr_entry *)(it->kaddr + it->ofs);
+	it->ofs += sizeof(struct erofs_xattr_entry);
+	value_sz = le16_to_cpu(entry.e_value_size);
+
+	h = erofs_xattr_handler(entry.e_name_index);
+	if (!h || (h->list && !h->list(it->dentry))) {
+		it->ofs += entry.e_name_len + value_sz;
+		goto out;
+	}
 
 	prefix = xattr_prefix(h);
 	prefix_len = strlen(prefix);
+	name_sz = prefix_len + entry.e_name_len + 1;
 
 	if (!it->buffer) {
-		it->buffer_ofs += prefix_len + entry->e_name_len + 1;
-		return 1;
+		it->buffer_ofs += name_sz;
+		it->ofs += entry.e_name_len + value_sz;
+		goto out;
 	}
-
-	if (it->buffer_ofs + prefix_len
-		+ entry->e_name_len + 1 > it->buffer_size)
+	if (it->buffer_ofs + name_sz > it->buffer_size)
 		return -ERANGE;
 
 	memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
 	it->buffer_ofs += prefix_len;
-	return 0;
-}
 
-static int xattr_namelist(struct erofs_xattr_iter *it,
-			  unsigned int processed, char *buf, unsigned int len)
-{
-	memcpy(it->buffer + it->buffer_ofs, buf, len);
-	it->buffer_ofs += len;
+	/* 2. handle xattr name (err can't be ENOATTR) */
+	err = erofs_xattr_body(it, entry.e_name_len, erofs_xattr_copy);
+	if (err)
+		return err;
+
+	it->buffer[it->buffer_ofs++] = '\0';
+	it->ofs += value_sz;
+out:
+	it->ofs = EROFS_XATTR_ALIGN(it->ofs);
 	return 0;
 }
 
-static int xattr_skipvalue(struct erofs_xattr_iter *it,
-			   unsigned int value_sz)
+static int erofs_getxattr_foreach(struct erofs_xattr_iter *it)
 {
-	it->buffer[it->buffer_ofs++] = '\0';
-	return 1;
-}
+	struct erofs_xattr_entry entry;
+	unsigned int value_sz;
+	int err;
 
-static const struct xattr_iter_handlers list_xattr_handlers = {
-	.entry = xattr_entrylist,
-	.name = xattr_namelist,
-	.alloc_buffer = xattr_skipvalue,
-	.value = NULL
-};
+	err = erofs_xattr_check_entry(it);
+	if (err)
+		return err;
+
+	/* 1. handle xattr entry */
+	entry = *(struct erofs_xattr_entry *)(it->kaddr + it->ofs);
+	it->ofs += sizeof(struct erofs_xattr_entry);
+	value_sz = le16_to_cpu(entry.e_value_size);
+
+	if (it->index != entry.e_name_index || it->name.len != entry.e_name_len) {
+		it->ofs += entry.e_name_len + value_sz;
+		err = -ENOATTR;
+		goto out;
+	}
+
+	/* 2. handle xattr name */
+	err = erofs_xattr_body(it, entry.e_name_len, erofs_xattr_namematch);
+	if (err) {
+		it->ofs += value_sz;
+		goto out;
+	}
+
+	/* 3. handle xattr value */
+	if (!it->buffer) {
+		it->buffer_ofs = value_sz;
+		it->ofs += value_sz;
+		goto out; /* err == 0 */
+	}
+	if (it->buffer_size < value_sz)
+		return -ERANGE;
+
+	/* no need updating ofs on error (err can't be ENOATTR) */
+	err = erofs_xattr_body(it, value_sz, erofs_xattr_copy);
+out:
+	it->ofs = EROFS_XATTR_ALIGN(it->ofs);
+	return err;
+}
 
 static int erofs_iter_inline_xattr(struct erofs_xattr_iter *it)
 {
 	struct erofs_inode *const vi = EROFS_I(it->inode);
-	const struct xattr_iter_handlers *op;
-	unsigned int xattr_header_sz, remaining;
+	unsigned int xattr_header_sz;
 	erofs_off_t pos;
 	int ret;
 
@@ -417,11 +387,12 @@ static int erofs_iter_inline_xattr(struct erofs_xattr_iter *it)
 	if (IS_ERR(it->kaddr))
 		return PTR_ERR(it->kaddr);
 
-	remaining = vi->xattr_isize - xattr_header_sz;
-	op = it->getxattr ? &find_xattr_handlers : &list_xattr_handlers;
-
-	while (remaining) {
-		ret = xattr_foreach(it, op, &remaining);
+	it->remaining = vi->xattr_isize - xattr_header_sz;
+	while (it->remaining) {
+		if (it->getxattr)
+			ret = erofs_getxattr_foreach(it);
+		else
+			ret = erofs_listxattr_foreach(it);
 		if ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
 			break;
 	}
@@ -432,12 +403,9 @@ static int erofs_iter_shared_xattr(struct erofs_xattr_iter *it)
 {
 	struct erofs_inode *const vi = EROFS_I(it->inode);
 	struct super_block *const sb = it->sb;
-	const struct xattr_iter_handlers *op;
 	unsigned int i, xsid;
 	int ret = -ENOATTR;
 
-	op = it->getxattr ? &find_xattr_handlers : &list_xattr_handlers;
-
 	for (i = 0; i < vi->xattr_shared_count; ++i) {
 		xsid = vi->xattr_shared_xattrs[i];
 		it->blkaddr = EROFS_SB(sb)->xattr_blkaddr +
@@ -447,7 +415,10 @@ static int erofs_iter_shared_xattr(struct erofs_xattr_iter *it)
 		if (IS_ERR(it->kaddr))
 			return PTR_ERR(it->kaddr);
 
-		ret = xattr_foreach(it, op, NULL);
+		if (it->getxattr)
+			ret = erofs_getxattr_foreach(it);
+		else
+			ret = erofs_listxattr_foreach(it);
 		if ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
 			break;
 	}
-- 
2.19.1.6.gb485710b

Re: [PATCH v2 1/8] erofs: move several xattr helpers into xattr.c

[Gao Xiang]

On 2023/3/30 16:29, Jingbo Xu wrote:
> Move xattrblock_addr() and xattrblock_offset() helpers into xattr.c,
> as they are not used outside of xattr.c.
> 
> inlinexattr_header_size() has only one caller, and thus make it inlined
> into the caller directly.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

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

Thanks,
Gao Xiang

Re: [PATCH v2 3/8] erofs: simplify erofs_xattr_generic_get()

[Gao Xiang]

On 2023/3/30 16:29, Jingbo Xu wrote:
> erofs_xattr_generic_get() won't be called from xattr handlers other than
> user/trusted/security xattr handler, and thus there's no need of extra
> checking.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

I plan to apply these three patches to -next first, and then look into
the rests.

Thanks,
Gao Xiang

> ---
>   fs/erofs/xattr.c | 17 +++--------------
>   1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index dc36a0c0919c..d76b74ece2e5 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -432,20 +432,9 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
>   				   struct dentry *unused, struct inode *inode,
>   				   const char *name, void *buffer, size_t size)
>   {
> -	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
> -
> -	switch (handler->flags) {
> -	case EROFS_XATTR_INDEX_USER:
> -		if (!test_opt(&sbi->opt, XATTR_USER))
> -			return -EOPNOTSUPP;
> -		break;
> -	case EROFS_XATTR_INDEX_TRUSTED:
> -		break;
> -	case EROFS_XATTR_INDEX_SECURITY:
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> +	if (handler->flags == EROFS_XATTR_INDEX_USER &&
> +	    !test_opt(&EROFS_I_SB(inode)->opt, XATTR_USER))
> +		return -EOPNOTSUPP;
>   
>   	return erofs_getxattr(inode, handler->flags, name, buffer, size);
>   }

Re: [PATCH v2 1/8] erofs: move several xattr helpers into xattr.c

[Yue Hu]

On Thu, 30 Mar 2023 16:29:03 +0800
Jingbo Xu <jefflexu@linux.alibaba.com> wrote:

> Move xattrblock_addr() and xattrblock_offset() helpers into xattr.c,
> as they are not used outside of xattr.c.
> 
> inlinexattr_header_size() has only one caller, and thus make it inlined
> into the caller directly.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

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

> ---
>  fs/erofs/xattr.c | 48 +++++++++++++++++++++++++++++-------------------
>  fs/erofs/xattr.h | 23 -----------------------
>  2 files changed, 29 insertions(+), 42 deletions(-)
> 
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index 459caa3cd65d..9ccd57581bc7 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -7,6 +7,19 @@
>  #include <linux/security.h>
>  #include "xattr.h"
>  
> +static inline erofs_blk_t erofs_xattr_blkaddr(struct super_block *sb,
> +					      unsigned int xattr_id)
> +{
> +	return EROFS_SB(sb)->xattr_blkaddr +
> +	       erofs_blknr(sb, xattr_id * sizeof(__u32));
> +}
> +
> +static inline unsigned int erofs_xattr_blkoff(struct super_block *sb,
> +					      unsigned int xattr_id)
> +{
> +	return erofs_blkoff(sb, xattr_id * sizeof(__u32));
> +}
> +
>  struct xattr_iter {
>  	struct super_block *sb;
>  	struct erofs_buf buf;
> @@ -157,7 +170,8 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
>  	struct erofs_inode *const vi = EROFS_I(inode);
>  	unsigned int xattr_header_sz, inline_xattr_ofs;
>  
> -	xattr_header_sz = inlinexattr_header_size(inode);
> +	xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
> +			  sizeof(u32) * vi->xattr_shared_count;
>  	if (xattr_header_sz >= vi->xattr_isize) {
>  		DBG_BUGON(xattr_header_sz > vi->xattr_isize);
>  		return -ENOATTR;
> @@ -351,20 +365,18 @@ static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
>  static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
>  {
>  	struct erofs_inode *const vi = EROFS_I(inode);
> -	struct super_block *const sb = inode->i_sb;
> -	unsigned int i;
> +	struct super_block *const sb = it->it.sb;
> +	unsigned int i, xsid;
>  	int ret = -ENOATTR;
>  
>  	for (i = 0; i < vi->xattr_shared_count; ++i) {
> -		erofs_blk_t blkaddr =
> -			xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
> -
> -		it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
> -		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
> -						  EROFS_KMAP);
> +		xsid = vi->xattr_shared_xattrs[i];
> +		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
> +		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
> +		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb,
> +						  it->it.blkaddr, EROFS_KMAP);
>  		if (IS_ERR(it->it.kaddr))
>  			return PTR_ERR(it->it.kaddr);
> -		it->it.blkaddr = blkaddr;
>  
>  		ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
>  		if (ret != -ENOATTR)
> @@ -562,20 +574,18 @@ static int shared_listxattr(struct listxattr_iter *it)
>  {
>  	struct inode *const inode = d_inode(it->dentry);
>  	struct erofs_inode *const vi = EROFS_I(inode);
> -	struct super_block *const sb = inode->i_sb;
> -	unsigned int i;
> +	struct super_block *const sb = it->it.sb;
> +	unsigned int i, xsid;
>  	int ret = 0;
>  
>  	for (i = 0; i < vi->xattr_shared_count; ++i) {
> -		erofs_blk_t blkaddr =
> -			xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
> -
> -		it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
> -		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
> -						  EROFS_KMAP);
> +		xsid = vi->xattr_shared_xattrs[i];
> +		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
> +		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
> +		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb,
> +						  it->it.blkaddr, EROFS_KMAP);
>  		if (IS_ERR(it->it.kaddr))
>  			return PTR_ERR(it->it.kaddr);
> -		it->it.blkaddr = blkaddr;
>  
>  		ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
>  		if (ret)
> diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
> index f7a21aaa9755..a65158cba14f 100644
> --- a/fs/erofs/xattr.h
> +++ b/fs/erofs/xattr.h
> @@ -13,29 +13,6 @@
>  /* Attribute not found */
>  #define ENOATTR         ENODATA
>  
> -static inline unsigned int inlinexattr_header_size(struct inode *inode)
> -{
> -	return sizeof(struct erofs_xattr_ibody_header) +
> -		sizeof(u32) * EROFS_I(inode)->xattr_shared_count;
> -}
> -
> -static inline erofs_blk_t xattrblock_addr(struct super_block *sb,
> -					  unsigned int xattr_id)
> -{
> -#ifdef CONFIG_EROFS_FS_XATTR
> -	return EROFS_SB(sb)->xattr_blkaddr +
> -		xattr_id * sizeof(__u32) / sb->s_blocksize;
> -#else
> -	return 0;
> -#endif
> -}
> -
> -static inline unsigned int xattrblock_offset(struct super_block *sb,
> -					     unsigned int xattr_id)
> -{
> -	return (xattr_id * sizeof(__u32)) % sb->s_blocksize;
> -}
> -
>  #ifdef CONFIG_EROFS_FS_XATTR
>  extern const struct xattr_handler erofs_xattr_user_handler;
>  extern const struct xattr_handler erofs_xattr_trusted_handler;

Re: [PATCH v2 2/8] erofs: rename init_inode_xattrs with erofs_ prefix

[Yue Hu]

On Thu, 30 Mar 2023 16:29:04 +0800
Jingbo Xu <jefflexu@linux.alibaba.com> wrote:

> Rename init_inode_xattrs() to erofs_init_inode_xattrs() without logic
> change.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

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

> ---
>  fs/erofs/xattr.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index 9ccd57581bc7..dc36a0c0919c 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -29,7 +29,7 @@ struct xattr_iter {
>  	unsigned int ofs;
>  };
>  
> -static int init_inode_xattrs(struct inode *inode)
> +static int erofs_init_inode_xattrs(struct inode *inode)
>  {
>  	struct erofs_inode *const vi = EROFS_I(inode);
>  	struct xattr_iter it;
> @@ -405,7 +405,7 @@ int erofs_getxattr(struct inode *inode, int index,
>  	if (!name)
>  		return -EINVAL;
>  
> -	ret = init_inode_xattrs(inode);
> +	ret = erofs_init_inode_xattrs(inode);
>  	if (ret)
>  		return ret;
>  
> @@ -600,7 +600,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
>  	int ret;
>  	struct listxattr_iter it;
>  
> -	ret = init_inode_xattrs(d_inode(dentry));
> +	ret = erofs_init_inode_xattrs(d_inode(dentry));
>  	if (ret == -ENOATTR)
>  		return 0;
>  	if (ret)

Re: [PATCH v2 3/8] erofs: simplify erofs_xattr_generic_get()

[Yue Hu]

On Thu, 30 Mar 2023 16:29:05 +0800
Jingbo Xu <jefflexu@linux.alibaba.com> wrote:

> erofs_xattr_generic_get() won't be called from xattr handlers other than
> user/trusted/security xattr handler, and thus there's no need of extra
> checking.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

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

> ---
>  fs/erofs/xattr.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index dc36a0c0919c..d76b74ece2e5 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -432,20 +432,9 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
>  				   struct dentry *unused, struct inode *inode,
>  				   const char *name, void *buffer, size_t size)
>  {
> -	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
> -
> -	switch (handler->flags) {
> -	case EROFS_XATTR_INDEX_USER:
> -		if (!test_opt(&sbi->opt, XATTR_USER))
> -			return -EOPNOTSUPP;
> -		break;
> -	case EROFS_XATTR_INDEX_TRUSTED:
> -		break;
> -	case EROFS_XATTR_INDEX_SECURITY:
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> +	if (handler->flags == EROFS_XATTR_INDEX_USER &&
> +	    !test_opt(&EROFS_I_SB(inode)->opt, XATTR_USER))
> +		return -EOPNOTSUPP;
>  
>  	return erofs_getxattr(inode, handler->flags, name, buffer, size);
>  }

Re: [PATCH v2 1/8] erofs: move several xattr helpers into xattr.c

[Chao Yu]

On 2023/3/30 16:29, Jingbo Xu wrote:
> Move xattrblock_addr() and xattrblock_offset() helpers into xattr.c,
> as they are not used outside of xattr.c.
> 
> inlinexattr_header_size() has only one caller, and thus make it inlined
> into the caller directly.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

Acked-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [PATCH v2 2/8] erofs: rename init_inode_xattrs with erofs_ prefix

[Chao Yu]

On 2023/3/30 16:29, Jingbo Xu wrote:
> Rename init_inode_xattrs() to erofs_init_inode_xattrs() without logic
> change.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Acked-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [PATCH v2 3/8] erofs: simplify erofs_xattr_generic_get()

[Chao Yu]

On 2023/3/30 16:29, Jingbo Xu wrote:
> erofs_xattr_generic_get() won't be called from xattr handlers other than
> user/trusted/security xattr handler, and thus there's no need of extra
> checking.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,