[PATCH 1/3] erofs-utils: get rid of erofs_buf_write_bhops

[PATCH 1/3] erofs-utils: get rid of erofs_buf_write_bhops

[Gao Xiang]

`nbh->off - bh->off` in erofs_bh_flush_generic_write() is
problematic due to erofs_bdrop(bh, false).

Let's avoid generic erofs_buf_write_bhops instead.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 include/erofs/cache.h |  1 -
 lib/cache.c           | 23 -----------------------
 mkfs/main.c           |  8 +++++---
 3 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/include/erofs/cache.h b/include/erofs/cache.h
index b04eb47..8c3bd46 100644
--- a/include/erofs/cache.h
+++ b/include/erofs/cache.h
@@ -80,7 +80,6 @@ static inline const int get_alignsize(int type, int *type_ret)
 
 extern const struct erofs_bhops erofs_drop_directly_bhops;
 extern const struct erofs_bhops erofs_skip_write_bhops;
-extern const struct erofs_bhops erofs_buf_write_bhops;
 
 static inline erofs_off_t erofs_btell(struct erofs_buffer_head *bh, bool end)
 {
diff --git a/lib/cache.c b/lib/cache.c
index 9eb0394..178bd5a 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -39,29 +39,6 @@ const struct erofs_bhops erofs_skip_write_bhops = {
 	.flush = erofs_bh_flush_skip_write,
 };
 
-int erofs_bh_flush_generic_write(struct erofs_buffer_head *bh, void *buf)
-{
-	struct erofs_buffer_head *nbh = list_next_entry(bh, list);
-	erofs_off_t offset = erofs_btell(bh, false);
-
-	DBG_BUGON(nbh->off < bh->off);
-	return dev_write(buf, offset, nbh->off - bh->off);
-}
-
-static bool erofs_bh_flush_buf_write(struct erofs_buffer_head *bh)
-{
-	int err = erofs_bh_flush_generic_write(bh, bh->fsprivate);
-
-	if (err)
-		return false;
-	free(bh->fsprivate);
-	return erofs_bh_flush_generic_end(bh);
-}
-
-const struct erofs_bhops erofs_buf_write_bhops = {
-	.flush = erofs_bh_flush_buf_write,
-};
-
 /* return buffer_head of erofs super block (with size 0) */
 struct erofs_buffer_head *erofs_buffer_init(void)
 {
diff --git a/mkfs/main.c b/mkfs/main.c
index 65d3df6..05db4c8 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -580,6 +580,7 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
 	};
 	const u32 sb_blksize = round_up(EROFS_SUPER_END, erofs_blksiz());
 	char *buf;
+	int ret;
 
 	*blocks         = erofs_mapbh(NULL);
 	sb.blocks       = cpu_to_le32(*blocks);
@@ -601,9 +602,10 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
 	}
 	memcpy(buf + EROFS_SUPER_OFFSET, &sb, sizeof(sb));
 
-	bh->fsprivate = buf;
-	bh->op = &erofs_buf_write_bhops;
-	return 0;
+	ret = dev_write(buf, erofs_btell(bh, false), sb_blksize);
+	free(buf);
+	erofs_bdrop(bh, false);
+	return ret;
 }
 
 static int erofs_mkfs_superblock_csum_set(void)
-- 
2.24.4

[PATCH 2/3] erofs-utils: xattr: avoid global variable shared_xattrs_size

[Gao Xiang]

Let's calculate shared_xattrs_size lazily instead.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 lib/xattr.c | 65 ++++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 38 deletions(-)

diff --git a/lib/xattr.c b/lib/xattr.c
index a292f2c..5f11cbe 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -37,7 +37,7 @@ struct inode_xattr_node {
 static DECLARE_HASHTABLE(ea_hashtable, EA_HASHTABLE_BITS);
 
 static LIST_HEAD(shared_xattrs_list);
-static unsigned int shared_xattrs_count, shared_xattrs_size;
+static unsigned int shared_xattrs_count;
 
 static struct xattr_prefix {
 	const char *prefix;
@@ -269,10 +269,6 @@ static int shared_xattr_add(struct xattr_item *item)
 	init_list_head(&node->list);
 	node->item = item;
 	list_add(&node->list, &shared_xattrs_list);
-
-	shared_xattrs_size += sizeof(struct erofs_xattr_entry);
-	shared_xattrs_size = EROFS_XATTR_ALIGN(shared_xattrs_size +
-					       item->len[0] + item->len[1]);
 	return ++shared_xattrs_count;
 }
 
@@ -543,24 +539,9 @@ static void erofs_cleanxattrs(bool sharedxattrs)
 	if (sharedxattrs)
 		return;
 
-	shared_xattrs_size = shared_xattrs_count = 0;
-}
-
-static bool erofs_bh_flush_write_shared_xattrs(struct erofs_buffer_head *bh)
-{
-	void *buf = bh->fsprivate;
-	int err = dev_write(buf, erofs_btell(bh, false), shared_xattrs_size);
-
-	if (err)
-		return false;
-	free(buf);
-	return erofs_bh_flush_generic_end(bh);
+	shared_xattrs_count = 0;
 }
 
-static struct erofs_bhops erofs_write_shared_xattrs_bhops = {
-	.flush = erofs_bh_flush_write_shared_xattrs,
-};
-
 static int comp_xattr_item(const void *a, const void *b)
 {
 	const struct xattr_item *ia, *ib;
@@ -587,13 +568,14 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 	char *buf;
 	unsigned int p, i;
 	erofs_off_t off;
+	erofs_off_t shared_xattrs_size = 0;
 
 	/* check if xattr or shared xattr is disabled */
 	if (cfg.c_inline_xattr_tolerance < 0 ||
 	    cfg.c_inline_xattr_tolerance == INT_MAX)
 		return 0;
 
-	if (shared_xattrs_size || shared_xattrs_count) {
+	if (shared_xattrs_count) {
 		DBG_BUGON(1);
 		return -EINVAL;
 	}
@@ -602,9 +584,26 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 	if (ret)
 		return ret;
 
-	if (!shared_xattrs_size)
+	if (!shared_xattrs_count)
 		goto out;
 
+	sorted_n = malloc(shared_xattrs_count * sizeof(n));
+	if (!sorted_n)
+		return -ENOMEM;
+
+	i = 0;
+	list_for_each_entry_safe(node, n, &shared_xattrs_list, list) {
+		list_del(&node->list);
+		sorted_n[i++] = node;
+
+		shared_xattrs_size += sizeof(struct erofs_xattr_entry);
+		shared_xattrs_size = EROFS_XATTR_ALIGN(shared_xattrs_size +
+				node->item->len[0] + node->item->len[1]);
+
+	}
+	DBG_BUGON(i != shared_xattrs_count);
+	qsort(sorted_n, shared_xattrs_count, sizeof(n), comp_xattr_item);
+
 	buf = calloc(1, shared_xattrs_size);
 	if (!buf)
 		return -ENOMEM;
@@ -622,18 +621,6 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 	sbi.xattr_blkaddr = off / erofs_blksiz();
 	off %= erofs_blksiz();
 	p = 0;
-
-	sorted_n = malloc(shared_xattrs_count * sizeof(n));
-	if (!sorted_n)
-		return -ENOMEM;
-	i = 0;
-	list_for_each_entry_safe(node, n, &shared_xattrs_list, list) {
-		list_del(&node->list);
-		sorted_n[i++] = node;
-	}
-	DBG_BUGON(i != shared_xattrs_count);
-	qsort(sorted_n, shared_xattrs_count, sizeof(n), comp_xattr_item);
-
 	for (i = 0; i < shared_xattrs_count; i++) {
 		struct inode_xattr_node *const tnode = sorted_n[i];
 		struct xattr_item *const item = tnode->item;
@@ -654,11 +641,13 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 	}
 
 	free(sorted_n);
-	bh->fsprivate = buf;
-	bh->op = &erofs_write_shared_xattrs_bhops;
+	bh->op = &erofs_drop_directly_bhops;
+	ret = dev_write(buf, erofs_btell(bh, false), shared_xattrs_size);
+	free(buf);
+	erofs_bdrop(bh, false);
 out:
 	erofs_cleanxattrs(true);
-	return 0;
+	return ret;
 }
 
 char *erofs_export_xattr_ibody(struct list_head *ixattrs, unsigned int size)
-- 
2.24.4

[PATCH 3/3] erofs-utils: xattr: avoid using inode_xattr_node for shared xattrs

[Gao Xiang]

Let's introduce next_shared_xattr instead to chain shared xattrs,
and it will also be used for ranged shared xattrs.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 lib/xattr.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/lib/xattr.c b/lib/xattr.c
index 5f11cbe..6034e7b 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -22,6 +22,7 @@
 #define EA_HASHTABLE_BITS 16
 
 struct xattr_item {
+	struct xattr_item *next_shared_xattr;
 	const char *kvbuf;
 	unsigned int hash[2], len[2], count;
 	int shared_xattr_id;
@@ -36,7 +37,7 @@ struct inode_xattr_node {
 
 static DECLARE_HASHTABLE(ea_hashtable, EA_HASHTABLE_BITS);
 
-static LIST_HEAD(shared_xattrs_list);
+static struct xattr_item *shared_xattrs_list;
 static unsigned int shared_xattrs_count;
 
 static struct xattr_prefix {
@@ -261,14 +262,8 @@ static int inode_xattr_add(struct list_head *hlist, struct xattr_item *item)
 
 static int shared_xattr_add(struct xattr_item *item)
 {
-	struct inode_xattr_node *node = malloc(sizeof(*node));
-
-	if (!node)
-		return -ENOMEM;
-
-	init_list_head(&node->list);
-	node->item = item;
-	list_add(&node->list, &shared_xattrs_list);
+	item->next_shared_xattr = shared_xattrs_list;
+	shared_xattrs_list = item;
 	return ++shared_xattrs_count;
 }
 
@@ -542,14 +537,14 @@ static void erofs_cleanxattrs(bool sharedxattrs)
 	shared_xattrs_count = 0;
 }
 
-static int comp_xattr_item(const void *a, const void *b)
+static int comp_shared_xattr_item(const void *a, const void *b)
 {
 	const struct xattr_item *ia, *ib;
 	unsigned int la, lb;
 	int ret;
 
-	ia = (*((const struct inode_xattr_node **)a))->item;
-	ib = (*((const struct inode_xattr_node **)b))->item;
+	ia = *((const struct xattr_item **)a);
+	ib = *((const struct xattr_item **)b);
 	la = ia->len[0] + ia->len[1];
 	lb = ib->len[0] + ib->len[1];
 
@@ -564,7 +559,7 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 {
 	int ret;
 	struct erofs_buffer_head *bh;
-	struct inode_xattr_node *node, *n, **sorted_n;
+	struct xattr_item *n, **sorted_n;
 	char *buf;
 	unsigned int p, i;
 	erofs_off_t off;
@@ -587,22 +582,23 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 	if (!shared_xattrs_count)
 		goto out;
 
-	sorted_n = malloc(shared_xattrs_count * sizeof(n));
+	sorted_n = malloc((shared_xattrs_count + 1) * sizeof(n));
 	if (!sorted_n)
 		return -ENOMEM;
 
 	i = 0;
-	list_for_each_entry_safe(node, n, &shared_xattrs_list, list) {
-		list_del(&node->list);
-		sorted_n[i++] = node;
+	while (shared_xattrs_list) {
+		struct xattr_item *item = shared_xattrs_list;
 
+		sorted_n[i++] = item;
+		shared_xattrs_list = item->next_shared_xattr;
 		shared_xattrs_size += sizeof(struct erofs_xattr_entry);
 		shared_xattrs_size = EROFS_XATTR_ALIGN(shared_xattrs_size +
-				node->item->len[0] + node->item->len[1]);
-
+				item->len[0] + item->len[1]);
 	}
 	DBG_BUGON(i != shared_xattrs_count);
-	qsort(sorted_n, shared_xattrs_count, sizeof(n), comp_xattr_item);
+	sorted_n[i] = NULL;
+	qsort(sorted_n, shared_xattrs_count, sizeof(n), comp_shared_xattr_item);
 
 	buf = calloc(1, shared_xattrs_size);
 	if (!buf)
@@ -622,14 +618,14 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 	off %= erofs_blksiz();
 	p = 0;
 	for (i = 0; i < shared_xattrs_count; i++) {
-		struct inode_xattr_node *const tnode = sorted_n[i];
-		struct xattr_item *const item = tnode->item;
+		struct xattr_item *item = sorted_n[i];
 		const struct erofs_xattr_entry entry = {
 			.e_name_index = item->prefix,
 			.e_name_len = item->len[0],
 			.e_value_size = cpu_to_le16(item->len[1])
 		};
 
+		item->next_shared_xattr = sorted_n[i + 1];
 		item->shared_xattr_id = (off + p) /
 			sizeof(struct erofs_xattr_entry);
 
@@ -637,9 +633,8 @@ int erofs_build_shared_xattrs_from_path(const char *path)
 		p += sizeof(struct erofs_xattr_entry);
 		memcpy(buf + p, item->kvbuf, item->len[0] + item->len[1]);
 		p = EROFS_XATTR_ALIGN(p + item->len[0] + item->len[1]);
-		free(tnode);
 	}
-
+	shared_xattrs_list = sorted_n[0];
 	free(sorted_n);
 	bh->op = &erofs_drop_directly_bhops;
 	ret = dev_write(buf, erofs_btell(bh, false), shared_xattrs_size);
-- 
2.24.4

[PATCH v2 1/3] erofs-utils: get rid of erofs_buf_write_bhops

[Gao Xiang]

`nbh->off - bh->off` in erofs_bh_flush_generic_write() is
problematic due to erofs_bdrop(bh, false).

Let's avoid generic erofs_buf_write_bhops instead.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
changes since v1:
 - should use EROFS_SUPER_END instead of sb_blksize, otherwise it
   could cause overlapping.

 include/erofs/cache.h |  1 -
 lib/cache.c           | 23 -----------------------
 mkfs/main.c           |  8 +++++---
 3 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/include/erofs/cache.h b/include/erofs/cache.h
index b04eb47..8c3bd46 100644
--- a/include/erofs/cache.h
+++ b/include/erofs/cache.h
@@ -80,7 +80,6 @@ static inline const int get_alignsize(int type, int *type_ret)
 
 extern const struct erofs_bhops erofs_drop_directly_bhops;
 extern const struct erofs_bhops erofs_skip_write_bhops;
-extern const struct erofs_bhops erofs_buf_write_bhops;
 
 static inline erofs_off_t erofs_btell(struct erofs_buffer_head *bh, bool end)
 {
diff --git a/lib/cache.c b/lib/cache.c
index 9eb0394..178bd5a 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -39,29 +39,6 @@ const struct erofs_bhops erofs_skip_write_bhops = {
 	.flush = erofs_bh_flush_skip_write,
 };
 
-int erofs_bh_flush_generic_write(struct erofs_buffer_head *bh, void *buf)
-{
-	struct erofs_buffer_head *nbh = list_next_entry(bh, list);
-	erofs_off_t offset = erofs_btell(bh, false);
-
-	DBG_BUGON(nbh->off < bh->off);
-	return dev_write(buf, offset, nbh->off - bh->off);
-}
-
-static bool erofs_bh_flush_buf_write(struct erofs_buffer_head *bh)
-{
-	int err = erofs_bh_flush_generic_write(bh, bh->fsprivate);
-
-	if (err)
-		return false;
-	free(bh->fsprivate);
-	return erofs_bh_flush_generic_end(bh);
-}
-
-const struct erofs_bhops erofs_buf_write_bhops = {
-	.flush = erofs_bh_flush_buf_write,
-};
-
 /* return buffer_head of erofs super block (with size 0) */
 struct erofs_buffer_head *erofs_buffer_init(void)
 {
diff --git a/mkfs/main.c b/mkfs/main.c
index 65d3df6..12e43c8 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -580,6 +580,7 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
 	};
 	const u32 sb_blksize = round_up(EROFS_SUPER_END, erofs_blksiz());
 	char *buf;
+	int ret;
 
 	*blocks         = erofs_mapbh(NULL);
 	sb.blocks       = cpu_to_le32(*blocks);
@@ -601,9 +602,10 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
 	}
 	memcpy(buf + EROFS_SUPER_OFFSET, &sb, sizeof(sb));
 
-	bh->fsprivate = buf;
-	bh->op = &erofs_buf_write_bhops;
-	return 0;
+	ret = dev_write(buf, erofs_btell(bh, false), EROFS_SUPER_END);
+	free(buf);
+	erofs_bdrop(bh, false);
+	return ret;
 }
 
 static int erofs_mkfs_superblock_csum_set(void)
-- 
2.24.4