linux-erofs.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] erofs-utils: get rid of inode->i_srcpath
@ 2021-04-05  9:49 Hu Weiwen
  2021-04-05 11:35 ` Gao Xiang
  0 siblings, 1 reply; 3+ messages in thread
From: Hu Weiwen @ 2021-04-05  9:49 UTC (permalink / raw)
  To: hsiangkao, linux-erofs

This cut the memory usage by half.

Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn>
---
 include/erofs/compress.h |  2 +-
 include/erofs/internal.h |  2 --
 include/erofs/xattr.h    |  2 +-
 lib/compress.c           | 24 +++++++--------------
 lib/inode.c              | 45 ++++++++++++++++++++--------------------
 lib/xattr.c              |  4 ++--
 6 files changed, 33 insertions(+), 46 deletions(-)

diff --git a/include/erofs/compress.h b/include/erofs/compress.h
index 952f287..a1dd55f 100644
--- a/include/erofs/compress.h
+++ b/include/erofs/compress.h
@@ -16,7 +16,7 @@
 #define EROFS_CONFIG_COMPR_MAX_SZ           (900  * 1024)
 #define EROFS_CONFIG_COMPR_MIN_SZ           (32   * 1024)
 
-int erofs_write_compressed_file(struct erofs_inode *inode);
+int erofs_write_compressed_file(struct erofs_inode *inode, int fd);
 
 int z_erofs_compress_init(void);
 int z_erofs_compress_exit(void);
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index ac5b270..d99d4ac 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -137,8 +137,6 @@ struct erofs_inode {
 		u32 i_rdev;
 	} u;
 
-	char i_srcpath[PATH_MAX + 1];
-
 	unsigned char datalayout;
 	unsigned char inode_isize;
 	/* inline tail-end packing size */
diff --git a/include/erofs/xattr.h b/include/erofs/xattr.h
index 197fe25..e22342d 100644
--- a/include/erofs/xattr.h
+++ b/include/erofs/xattr.h
@@ -42,7 +42,7 @@
 #define XATTR_NAME_POSIX_ACL_DEFAULT "system.posix_acl_default"
 #endif
 
-int erofs_prepare_xattr_ibody(struct erofs_inode *inode);
+int erofs_prepare_xattr_ibody(struct erofs_inode *inode, const char *path);
 char *erofs_export_xattr_ibody(struct list_head *ixattrs, unsigned int size);
 int erofs_build_shared_xattrs_from_path(const char *path);
 
diff --git a/lib/compress.c b/lib/compress.c
index 4b685cd..ea2310e 100644
--- a/lib/compress.c
+++ b/lib/compress.c
@@ -170,8 +170,7 @@ static int vle_compress_one(struct erofs_inode *inode,
 					      &count, dst, EROFS_BLKSIZ);
 		if (ret <= 0) {
 			if (ret != -EAGAIN) {
-				erofs_err("failed to compress %s: %s",
-					  inode->i_srcpath,
+				erofs_err("failed to compress: %s",
 					  erofs_strerror(ret));
 			}
 nocompression:
@@ -388,30 +387,24 @@ int z_erofs_convert_to_compacted_format(struct erofs_inode *inode,
 	return 0;
 }
 
-int erofs_write_compressed_file(struct erofs_inode *inode)
+int erofs_write_compressed_file(struct erofs_inode *inode, int fd)
 {
 	struct erofs_buffer_head *bh;
 	struct z_erofs_vle_compress_ctx ctx;
 	erofs_off_t remaining;
 	erofs_blk_t blkaddr, compressed_blocks;
 	unsigned int legacymetasize;
-	int ret, fd;
+	int ret;
 
 	u8 *compressmeta = malloc(vle_compressmeta_capacity(inode->i_size));
 	if (!compressmeta)
 		return -ENOMEM;
 
-	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
-	if (fd < 0) {
-		ret = -errno;
-		goto err_free;
-	}
-
 	/* allocate main data buffer */
 	bh = erofs_balloc(DATA, 0, 0, 0);
 	if (IS_ERR(bh)) {
 		ret = PTR_ERR(bh);
-		goto err_close;
+		goto err;
 	}
 
 	memset(compressmeta, 0, Z_EROFS_LEGACY_MAP_HEADER_SIZE);
@@ -455,13 +448,12 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
 
 	vle_write_indexes_final(&ctx);
 
-	close(fd);
 	DBG_BUGON(!compressed_blocks);
 	ret = erofs_bh_balloon(bh, blknr_to_addr(compressed_blocks));
 	DBG_BUGON(ret != EROFS_BLKSIZ);
 
-	erofs_info("compressed %s (%llu bytes) into %u blocks",
-		   inode->i_srcpath, (unsigned long long)inode->i_size,
+	erofs_info("compressed %llu bytes into %u blocks",
+		   (unsigned long long)inode->i_size,
 		   compressed_blocks);
 
 	/*
@@ -486,9 +478,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
 
 err_bdrop:
 	erofs_bdrop(bh, true);	/* revoke buffer */
-err_close:
-	close(fd);
-err_free:
+err:
 	free(compressmeta);
 	return ret;
 }
diff --git a/lib/inode.c b/lib/inode.c
index d52facf..44ed6f6 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -362,9 +362,9 @@ static int write_uncompressed_file_from_fd(struct erofs_inode *inode, int fd)
 	return 0;
 }
 
-int erofs_write_file(struct erofs_inode *inode)
+int erofs_write_file(struct erofs_inode *inode, int fd)
 {
-	int ret, fd;
+	int ret;
 
 	if (!inode->i_size) {
 		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
@@ -372,17 +372,15 @@ int erofs_write_file(struct erofs_inode *inode)
 	}
 
 	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
-		ret = erofs_write_compressed_file(inode);
+		ret = erofs_write_compressed_file(inode, fd);
 
-		if (!ret || ret != -ENOSPC)
+		if (!ret || ret != -ENOSPC) {
+			close(fd);
 			return ret;
+		}
 	}
 
 	/* fallback to all data uncompressed */
-	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
-	if (fd < 0)
-		return -errno;
-
 	ret = write_uncompressed_file_from_fd(inode, fd);
 	close(fd);
 	return ret;
@@ -786,16 +784,12 @@ int erofs_fill_inode(struct erofs_inode *inode,
 		return -EINVAL;
 	}
 
-	strncpy(inode->i_srcpath, path, sizeof(inode->i_srcpath) - 1);
-	inode->i_srcpath[sizeof(inode->i_srcpath) - 1] = '\0';
-
 	inode->dev = st->st_dev;
 	inode->i_ino[1] = st->st_ino;
 
 	if (erofs_should_use_inode_extended(inode)) {
 		if (cfg.c_force_inodeversion == FORCE_INODE_COMPACT) {
-			erofs_err("file %s cannot be in compact form",
-				  inode->i_srcpath);
+			erofs_err("file %s cannot be in compact form", path);
 			return -EINVAL;
 		}
 		inode->inode_isize = sizeof(struct erofs_inode_extended);
@@ -916,14 +910,15 @@ void erofs_d_invalidate(struct erofs_dentry *d)
 	erofs_iput(inode);
 }
 
-struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
+struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir,
+		const char *src_path)
 {
 	int ret;
 	DIR *_dir;
 	struct dirent *dp;
 	struct erofs_dentry *d;
 
-	ret = erofs_prepare_xattr_ibody(dir);
+	ret = erofs_prepare_xattr_ibody(dir, src_path);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
@@ -933,7 +928,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 
 			if (!symlink)
 				return ERR_PTR(-ENOMEM);
-			ret = readlink(dir->i_srcpath, symlink, dir->i_size);
+			ret = readlink(src_path, symlink, dir->i_size);
 			if (ret < 0) {
 				free(symlink);
 				return ERR_PTR(-errno);
@@ -944,7 +939,11 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 			if (ret)
 				return ERR_PTR(ret);
 		} else {
-			ret = erofs_write_file(dir);
+			int fd = open(src_path, O_RDONLY | O_BINARY);
+
+			if (fd < 0)
+				return ERR_PTR(-errno);
+			ret = erofs_write_file(dir, fd);
 			if (ret)
 				return ERR_PTR(ret);
 		}
@@ -954,10 +953,10 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 		return dir;
 	}
 
-	_dir = opendir(dir->i_srcpath);
+	_dir = opendir(src_path);
 	if (!_dir) {
 		erofs_err("%s, failed to opendir at %s: %s",
-			  __func__, dir->i_srcpath, erofs_strerror(errno));
+			  __func__, src_path, erofs_strerror(errno));
 		return ERR_PTR(-errno);
 	}
 
@@ -976,7 +975,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 			continue;
 
 		/* skip if it's a exclude file */
-		if (erofs_is_exclude_path(dir->i_srcpath, dp->d_name))
+		if (erofs_is_exclude_path(src_path, dp->d_name))
 			continue;
 
 		d = erofs_d_alloc(dir, dp->d_name);
@@ -1017,7 +1016,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
 		}
 
 		ret = snprintf(buf, PATH_MAX, "%s/%s",
-			       dir->i_srcpath, d->name);
+			       src_path, d->name);
 		if (ret < 0 || ret >= PATH_MAX) {
 			/* ignore the too long path */
 			goto fail;
@@ -1038,7 +1037,7 @@ fail:
 
 		erofs_d_invalidate(d);
 		erofs_info("add file %s/%s (nid %llu, type %d)",
-			   dir->i_srcpath, d->name, (unsigned long long)d->nid,
+			   src_path, d->name, (unsigned long long)d->nid,
 			   d->type);
 	}
 	erofs_write_dir_file(dir);
@@ -1071,6 +1070,6 @@ struct erofs_inode *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
 	else
 		inode->i_parent = inode;	/* rootdir mark */
 
-	return erofs_mkfs_build_tree(inode);
+	return erofs_mkfs_build_tree(inode, path);
 }
 
diff --git a/lib/xattr.c b/lib/xattr.c
index 8b7bcb1..e128f3d 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -401,7 +401,7 @@ static int erofs_droid_xattr_set_caps(struct erofs_inode *inode)
 }
 #endif
 
-int erofs_prepare_xattr_ibody(struct erofs_inode *inode)
+int erofs_prepare_xattr_ibody(struct erofs_inode *inode, const char *path)
 {
 	int ret;
 	struct inode_xattr_node *node;
@@ -411,7 +411,7 @@ int erofs_prepare_xattr_ibody(struct erofs_inode *inode)
 	if (cfg.c_inline_xattr_tolerance < 0)
 		return 0;
 
-	ret = read_xattrs_from_file(inode->i_srcpath, inode->i_mode, ixattrs);
+	ret = read_xattrs_from_file(path, inode->i_mode, ixattrs);
 	if (ret < 0)
 		return ret;
 
-- 
2.25.1


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

* Re: [PATCH] erofs-utils: get rid of inode->i_srcpath
  2021-04-05  9:49 [PATCH] erofs-utils: get rid of inode->i_srcpath Hu Weiwen
@ 2021-04-05 11:35 ` Gao Xiang
  2021-04-05 12:11   ` 胡玮文
  0 siblings, 1 reply; 3+ messages in thread
From: Gao Xiang @ 2021-04-05 11:35 UTC (permalink / raw)
  To: Hu Weiwen; +Cc: linux-erofs

Hi Weiwen,

On Mon, Apr 05, 2021 at 05:49:50PM +0800, Hu Weiwen wrote:
> This cut the memory usage by half.
> 
> Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn>
> ---

I have to hold on this patch for now, 3 main reasons here:
 1) I'd like to apply big pcluster first, which is a main new feature
    for erofs-utils 1.3;
 2) even though it saves memory usage (and not sure how much memory
    you need to save), I think srcpath is more useful for debugging
    and file tracing; Also, I think we could flush inode in advance to
    save memory usage (e.g. by using bflush()) as well.
 3) if src_path is too large, how about malloc() it as the first step,
    even that, I'd like to apply it after erofs-utils 1.3 is out...

Thanks,
Gao Xiang

>  include/erofs/compress.h |  2 +-
>  include/erofs/internal.h |  2 --
>  include/erofs/xattr.h    |  2 +-
>  lib/compress.c           | 24 +++++++--------------
>  lib/inode.c              | 45 ++++++++++++++++++++--------------------
>  lib/xattr.c              |  4 ++--
>  6 files changed, 33 insertions(+), 46 deletions(-)
> 
> diff --git a/include/erofs/compress.h b/include/erofs/compress.h
> index 952f287..a1dd55f 100644
> --- a/include/erofs/compress.h
> +++ b/include/erofs/compress.h
> @@ -16,7 +16,7 @@
>  #define EROFS_CONFIG_COMPR_MAX_SZ           (900  * 1024)
>  #define EROFS_CONFIG_COMPR_MIN_SZ           (32   * 1024)
>  
> -int erofs_write_compressed_file(struct erofs_inode *inode);
> +int erofs_write_compressed_file(struct erofs_inode *inode, int fd);
>  
>  int z_erofs_compress_init(void);
>  int z_erofs_compress_exit(void);
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index ac5b270..d99d4ac 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -137,8 +137,6 @@ struct erofs_inode {
>  		u32 i_rdev;
>  	} u;
>  
> -	char i_srcpath[PATH_MAX + 1];
> -
>  	unsigned char datalayout;
>  	unsigned char inode_isize;
>  	/* inline tail-end packing size */
> diff --git a/include/erofs/xattr.h b/include/erofs/xattr.h
> index 197fe25..e22342d 100644
> --- a/include/erofs/xattr.h
> +++ b/include/erofs/xattr.h
> @@ -42,7 +42,7 @@
>  #define XATTR_NAME_POSIX_ACL_DEFAULT "system.posix_acl_default"
>  #endif
>  
> -int erofs_prepare_xattr_ibody(struct erofs_inode *inode);
> +int erofs_prepare_xattr_ibody(struct erofs_inode *inode, const char *path);
>  char *erofs_export_xattr_ibody(struct list_head *ixattrs, unsigned int size);
>  int erofs_build_shared_xattrs_from_path(const char *path);
>  
> diff --git a/lib/compress.c b/lib/compress.c
> index 4b685cd..ea2310e 100644
> --- a/lib/compress.c
> +++ b/lib/compress.c
> @@ -170,8 +170,7 @@ static int vle_compress_one(struct erofs_inode *inode,
>  					      &count, dst, EROFS_BLKSIZ);
>  		if (ret <= 0) {
>  			if (ret != -EAGAIN) {
> -				erofs_err("failed to compress %s: %s",
> -					  inode->i_srcpath,
> +				erofs_err("failed to compress: %s",
>  					  erofs_strerror(ret));
>  			}
>  nocompression:
> @@ -388,30 +387,24 @@ int z_erofs_convert_to_compacted_format(struct erofs_inode *inode,
>  	return 0;
>  }
>  
> -int erofs_write_compressed_file(struct erofs_inode *inode)
> +int erofs_write_compressed_file(struct erofs_inode *inode, int fd)
>  {
>  	struct erofs_buffer_head *bh;
>  	struct z_erofs_vle_compress_ctx ctx;
>  	erofs_off_t remaining;
>  	erofs_blk_t blkaddr, compressed_blocks;
>  	unsigned int legacymetasize;
> -	int ret, fd;
> +	int ret;
>  
>  	u8 *compressmeta = malloc(vle_compressmeta_capacity(inode->i_size));
>  	if (!compressmeta)
>  		return -ENOMEM;
>  
> -	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> -	if (fd < 0) {
> -		ret = -errno;
> -		goto err_free;
> -	}
> -
>  	/* allocate main data buffer */
>  	bh = erofs_balloc(DATA, 0, 0, 0);
>  	if (IS_ERR(bh)) {
>  		ret = PTR_ERR(bh);
> -		goto err_close;
> +		goto err;
>  	}
>  
>  	memset(compressmeta, 0, Z_EROFS_LEGACY_MAP_HEADER_SIZE);
> @@ -455,13 +448,12 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
>  
>  	vle_write_indexes_final(&ctx);
>  
> -	close(fd);
>  	DBG_BUGON(!compressed_blocks);
>  	ret = erofs_bh_balloon(bh, blknr_to_addr(compressed_blocks));
>  	DBG_BUGON(ret != EROFS_BLKSIZ);
>  
> -	erofs_info("compressed %s (%llu bytes) into %u blocks",
> -		   inode->i_srcpath, (unsigned long long)inode->i_size,
> +	erofs_info("compressed %llu bytes into %u blocks",
> +		   (unsigned long long)inode->i_size,
>  		   compressed_blocks);
>  
>  	/*
> @@ -486,9 +478,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
>  
>  err_bdrop:
>  	erofs_bdrop(bh, true);	/* revoke buffer */
> -err_close:
> -	close(fd);
> -err_free:
> +err:
>  	free(compressmeta);
>  	return ret;
>  }
> diff --git a/lib/inode.c b/lib/inode.c
> index d52facf..44ed6f6 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -362,9 +362,9 @@ static int write_uncompressed_file_from_fd(struct erofs_inode *inode, int fd)
>  	return 0;
>  }
>  
> -int erofs_write_file(struct erofs_inode *inode)
> +int erofs_write_file(struct erofs_inode *inode, int fd)
>  {
> -	int ret, fd;
> +	int ret;
>  
>  	if (!inode->i_size) {
>  		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> @@ -372,17 +372,15 @@ int erofs_write_file(struct erofs_inode *inode)
>  	}
>  
>  	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
> -		ret = erofs_write_compressed_file(inode);
> +		ret = erofs_write_compressed_file(inode, fd);
>  
> -		if (!ret || ret != -ENOSPC)
> +		if (!ret || ret != -ENOSPC) {
> +			close(fd);
>  			return ret;
> +		}
>  	}
>  
>  	/* fallback to all data uncompressed */
> -	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> -	if (fd < 0)
> -		return -errno;
> -
>  	ret = write_uncompressed_file_from_fd(inode, fd);
>  	close(fd);
>  	return ret;
> @@ -786,16 +784,12 @@ int erofs_fill_inode(struct erofs_inode *inode,
>  		return -EINVAL;
>  	}
>  
> -	strncpy(inode->i_srcpath, path, sizeof(inode->i_srcpath) - 1);
> -	inode->i_srcpath[sizeof(inode->i_srcpath) - 1] = '\0';
> -
>  	inode->dev = st->st_dev;
>  	inode->i_ino[1] = st->st_ino;
>  
>  	if (erofs_should_use_inode_extended(inode)) {
>  		if (cfg.c_force_inodeversion == FORCE_INODE_COMPACT) {
> -			erofs_err("file %s cannot be in compact form",
> -				  inode->i_srcpath);
> +			erofs_err("file %s cannot be in compact form", path);
>  			return -EINVAL;
>  		}
>  		inode->inode_isize = sizeof(struct erofs_inode_extended);
> @@ -916,14 +910,15 @@ void erofs_d_invalidate(struct erofs_dentry *d)
>  	erofs_iput(inode);
>  }
>  
> -struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
> +struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir,
> +		const char *src_path)
>  {
>  	int ret;
>  	DIR *_dir;
>  	struct dirent *dp;
>  	struct erofs_dentry *d;
>  
> -	ret = erofs_prepare_xattr_ibody(dir);
> +	ret = erofs_prepare_xattr_ibody(dir, src_path);
>  	if (ret < 0)
>  		return ERR_PTR(ret);
>  
> @@ -933,7 +928,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  
>  			if (!symlink)
>  				return ERR_PTR(-ENOMEM);
> -			ret = readlink(dir->i_srcpath, symlink, dir->i_size);
> +			ret = readlink(src_path, symlink, dir->i_size);
>  			if (ret < 0) {
>  				free(symlink);
>  				return ERR_PTR(-errno);
> @@ -944,7 +939,11 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  			if (ret)
>  				return ERR_PTR(ret);
>  		} else {
> -			ret = erofs_write_file(dir);
> +			int fd = open(src_path, O_RDONLY | O_BINARY);
> +
> +			if (fd < 0)
> +				return ERR_PTR(-errno);
> +			ret = erofs_write_file(dir, fd);
>  			if (ret)
>  				return ERR_PTR(ret);
>  		}
> @@ -954,10 +953,10 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  		return dir;
>  	}
>  
> -	_dir = opendir(dir->i_srcpath);
> +	_dir = opendir(src_path);
>  	if (!_dir) {
>  		erofs_err("%s, failed to opendir at %s: %s",
> -			  __func__, dir->i_srcpath, erofs_strerror(errno));
> +			  __func__, src_path, erofs_strerror(errno));
>  		return ERR_PTR(-errno);
>  	}
>  
> @@ -976,7 +975,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  			continue;
>  
>  		/* skip if it's a exclude file */
> -		if (erofs_is_exclude_path(dir->i_srcpath, dp->d_name))
> +		if (erofs_is_exclude_path(src_path, dp->d_name))
>  			continue;
>  
>  		d = erofs_d_alloc(dir, dp->d_name);
> @@ -1017,7 +1016,7 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>  		}
>  
>  		ret = snprintf(buf, PATH_MAX, "%s/%s",
> -			       dir->i_srcpath, d->name);
> +			       src_path, d->name);
>  		if (ret < 0 || ret >= PATH_MAX) {
>  			/* ignore the too long path */
>  			goto fail;
> @@ -1038,7 +1037,7 @@ fail:
>  
>  		erofs_d_invalidate(d);
>  		erofs_info("add file %s/%s (nid %llu, type %d)",
> -			   dir->i_srcpath, d->name, (unsigned long long)d->nid,
> +			   src_path, d->name, (unsigned long long)d->nid,
>  			   d->type);
>  	}
>  	erofs_write_dir_file(dir);
> @@ -1071,6 +1070,6 @@ struct erofs_inode *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
>  	else
>  		inode->i_parent = inode;	/* rootdir mark */
>  
> -	return erofs_mkfs_build_tree(inode);
> +	return erofs_mkfs_build_tree(inode, path);
>  }
>  
> diff --git a/lib/xattr.c b/lib/xattr.c
> index 8b7bcb1..e128f3d 100644
> --- a/lib/xattr.c
> +++ b/lib/xattr.c
> @@ -401,7 +401,7 @@ static int erofs_droid_xattr_set_caps(struct erofs_inode *inode)
>  }
>  #endif
>  
> -int erofs_prepare_xattr_ibody(struct erofs_inode *inode)
> +int erofs_prepare_xattr_ibody(struct erofs_inode *inode, const char *path)
>  {
>  	int ret;
>  	struct inode_xattr_node *node;
> @@ -411,7 +411,7 @@ int erofs_prepare_xattr_ibody(struct erofs_inode *inode)
>  	if (cfg.c_inline_xattr_tolerance < 0)
>  		return 0;
>  
> -	ret = read_xattrs_from_file(inode->i_srcpath, inode->i_mode, ixattrs);
> +	ret = read_xattrs_from_file(path, inode->i_mode, ixattrs);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.25.1
> 


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

* Re: [PATCH] erofs-utils: get rid of inode->i_srcpath
  2021-04-05 11:35 ` Gao Xiang
@ 2021-04-05 12:11   ` 胡玮文
  0 siblings, 0 replies; 3+ messages in thread
From: 胡玮文 @ 2021-04-05 12:11 UTC (permalink / raw)
  To: Gao Xiang; +Cc: linux-erofs

On Mon, Apr 05, 2021 at 07:35:25PM +0800, Gao Xiang wrote:
> Hi Weiwen,
> 
> On Mon, Apr 05, 2021 at 05:49:50PM +0800, Hu Weiwen wrote:
> > This cut the memory usage by half.
> > 
> > Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn>
> > ---
> 
> I have to hold on this patch for now, 3 main reasons here:
>  1) I'd like to apply big pcluster first, which is a main new feature
>     for erofs-utils 1.3;

OK

>  2) even though it saves memory usage (and not sure how much memory
>     you need to save), I think srcpath is more useful for debugging
>     and file tracing; Also, I think we could flush inode in advance to
>     save memory usage (e.g. by using bflush()) as well.

I think flush in advance is a good idea. But it needs more investigation.
Ideally, I want to make the space complexity O(1), so that we can pack an
arbitrarily large number of files. It's better not to save every seen inode
structure in memory.

If I remember it correctly, when packing a dataset consisting of ~5m files,
mkfs.erofs takes 30-40GiB memory. I think this can hinder some use cases.

As for debugging, maybe we can create a routine that travels up the directory
tree to rebuild the path when needed? (Just like `pwd'.) Or, for mkfs, we can
just pass around a pointer to the current working path.

Thanks,
Hu Weiwen

>  3) if src_path is too large, how about malloc() it as the first step,
>     even that, I'd like to apply it after erofs-utils 1.3 is out...
> 
> Thanks,
> Gao Xiang


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

end of thread, other threads:[~2021-04-05 12:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-05  9:49 [PATCH] erofs-utils: get rid of inode->i_srcpath Hu Weiwen
2021-04-05 11:35 ` Gao Xiang
2021-04-05 12:11   ` 胡玮文

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