Linux-EROFS Archive on lore.kernel.org
 help / color / Atom feed
* [RFCv3] erofs-utils: on-disk extent format for blocks
@ 2020-01-02  9:47 Pratik Shinde
  2020-01-02 10:47 ` Gao Xiang
  0 siblings, 1 reply; 6+ messages in thread
From: Pratik Shinde @ 2020-01-02  9:47 UTC (permalink / raw)
  To: linux-erofs, bluce.liguifu, miaoxie, fangwei1

1)Moved on-disk structures to erofs_fs.h
2)Some naming changes.

I think we can keep 'IS_HOLE()' macro, otherwise the code
does not look clean(if used directly/without macro).Its getting
used only in inode.c so it can be kept there.
what do you think ?

Signed-off-by: Pratik Shinde <pratikshinde320@gmail.com>
---
 include/erofs/internal.h |   9 ++-
 include/erofs_fs.h       |  11 ++++
 lib/inode.c              | 153 ++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 150 insertions(+), 23 deletions(-)

diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index e13adda..2d7466b 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -63,7 +63,7 @@ struct erofs_sb_info {
 extern struct erofs_sb_info sbi;
 
 struct erofs_inode {
-	struct list_head i_hash, i_subdirs, i_xattrs;
+	struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;
 
 	unsigned int i_count;
 	struct erofs_inode *i_parent;
@@ -93,6 +93,7 @@ struct erofs_inode {
 
 	unsigned int xattr_isize;
 	unsigned int extent_isize;
+	unsigned int sparse_extent_isize;
 
 	erofs_nid_t nid;
 	struct erofs_buffer_head *bh;
@@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int err)
 	return msg;
 }
 
+struct erofs_sparse_extent_node {
+	struct list_head next;
+	erofs_blk_t lblk;
+	erofs_blk_t pblk;
+	u32 len;
+};
 #endif
 
diff --git a/include/erofs_fs.h b/include/erofs_fs.h
index bcc4f0c..a63e1c6 100644
--- a/include/erofs_fs.h
+++ b/include/erofs_fs.h
@@ -321,5 +321,16 @@ static inline void erofs_check_ondisk_layout_definitions(void)
 		     Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
 }
 
+/* on-disk sparse extent format */
+struct erofs_sparse_extent {
+	__le32 ee_lblk;
+	__le32 ee_pblk;
+	__le32 ee_len;
+};
+
+struct erofs_sparse_extent_iheader {
+	u32 count;
+};
+
 #endif
 
diff --git a/lib/inode.c b/lib/inode.c
index 0e19b11..da20599 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT >> S_SHIFT] = {
 
 struct list_head inode_hashtable[NR_INODE_HASHTABLE];
 
+
+#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start &&	\
+		             roundup(end, EROFS_BLKSIZ) == end &&	\
+			    (end - start) % EROFS_BLKSIZ == 0)
+
+/**
+   read extents of the given file.
+   record the data extents and link them into a chain.
+   exclude holes present in file.
+ */
+unsigned int erofs_read_sparse_extents(int fd, struct list_head *extents)
+{
+	erofs_blk_t startblk, endblk, datablk;
+	unsigned int nholes = 0;
+	erofs_off_t data, hole, len = 0, last_data;
+	struct erofs_sparse_extent_node *e_data;
+
+	len = lseek(fd, 0, SEEK_END);
+	if (len < 0)
+		return -errno;
+	if (lseek(fd, 0, SEEK_SET) == -1)
+		return -errno;
+	data = 0;
+	last_data = 0;
+	while (data < len) {
+		hole = lseek(fd, data, SEEK_HOLE);
+		if (hole == len)
+			break;
+		data = lseek(fd, hole, SEEK_DATA);
+		if (data < 0 || hole > data)
+			return -EINVAL;
+		if (IS_HOLE(hole, data)) {
+			startblk = erofs_blknr(hole);
+			datablk = erofs_blknr(last_data);
+			endblk = erofs_blknr(data);
+			last_data = data;
+			e_data = malloc(sizeof(
+					 struct erofs_sparse_extent_node));
+			if (e_data == NULL)
+				return -ENOMEM;
+			e_data->lblk = datablk;
+			e_data->len = (startblk - datablk);
+			list_add_tail(&e_data->next, extents);
+			nholes += (endblk - startblk);
+		}
+	}
+	/* rounddown to exclude tail-end data */
+	if (last_data < len && (len - last_data) >= EROFS_BLKSIZ) {
+		e_data = malloc(sizeof(struct erofs_sparse_extent_node));
+		if (e_data == NULL)
+			return -ENOMEM;
+		startblk = erofs_blknr(last_data);
+		e_data->lblk = startblk;
+		e_data->len = erofs_blknr(rounddown((len - last_data),
+					  EROFS_BLKSIZ));
+		list_add_tail(&e_data->next, extents);
+	}
+	return nholes;
+}
+
+int erofs_write_sparse_extents(struct erofs_inode *inode, erofs_off_t off)
+{
+	struct erofs_sparse_extent_node *e_node;
+	struct erofs_sparse_extent_iheader *header;
+	char *buf;
+	unsigned int p = 0;
+	int ret;
+
+	buf = malloc(inode->sparse_extent_isize);
+	if (buf == NULL)
+		return -ENOMEM;
+	header = (struct erofs_sparse_extent_iheader *) buf;
+	header->count = 0;
+	p += sizeof(struct erofs_sparse_extent_iheader);
+	list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
+		const struct erofs_sparse_extent ee = {
+			.ee_lblk = cpu_to_le32(e_node->lblk),
+			.ee_pblk = cpu_to_le32(e_node->pblk),
+			.ee_len  = cpu_to_le32(e_node->len)
+		};
+		memcpy(buf + p, &ee, sizeof(struct erofs_sparse_extent));
+		p += sizeof(struct erofs_sparse_extent);
+		header->count++;
+		list_del(&e_node->next);
+		free(e_node);
+	}
+	ret = dev_write(buf, off, inode->sparse_extent_isize);
+	free(buf);
+	return ret;
+}
+
 void erofs_inode_manager_init(void)
 {
 	unsigned int i;
@@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct erofs_inode *inode)
 
 int erofs_write_file(struct erofs_inode *inode)
 {
-	unsigned int nblocks, i;
+	unsigned int nblocks, i, j, nholes;
 	int ret, fd;
+	struct erofs_sparse_extent_node *e_node;
 
 	if (!inode->i_size) {
 		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
@@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)
 	/* fallback to all data uncompressed */
 	inode->datalayout = EROFS_INODE_FLAT_INLINE;
 	nblocks = inode->i_size / EROFS_BLKSIZ;
-
-	ret = __allocate_inode_bh_data(inode, nblocks);
-	if (ret)
-		return ret;
-
 	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
 	if (fd < 0)
 		return -errno;
-
-	for (i = 0; i < nblocks; ++i) {
-		char buf[EROFS_BLKSIZ];
-
-		ret = read(fd, buf, EROFS_BLKSIZ);
-		if (ret != EROFS_BLKSIZ) {
-			if (ret < 0)
+	nholes = erofs_read_sparse_extents(fd, &inode->i_sparse_extents);
+	if (nholes < 0) {
+		close(fd);
+		return nholes;
+	}
+	ret = __allocate_inode_bh_data(inode, nblocks - nholes);
+	if (ret) {
+		close(fd);
+		return ret;
+	}
+	i = inode->u.i_blkaddr;
+	inode->sparse_extent_isize = sizeof(struct erofs_sparse_extent_iheader);
+	list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
+		inode->sparse_extent_isize += sizeof(struct erofs_sparse_extent);
+		e_node->pblk = i;
+		ret = lseek(fd, blknr_to_addr(e_node->lblk), SEEK_SET);
+		if (ret < 0)
+			goto fail;
+		for (j = 0; j < e_node->len; j++) {
+			char buf[EROFS_BLKSIZ];
+			ret = read(fd, buf, EROFS_BLKSIZ);
+			if (ret != EROFS_BLKSIZ) {
+				if (ret < 0)
+					goto fail;
+				close(fd);
+				return -EAGAIN;
+			}
+			ret = blk_write(buf, e_node->pblk + j, 1);
+			if (ret)
 				goto fail;
-			close(fd);
-			return -EAGAIN;
 		}
-
-		ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
-		if (ret)
-			goto fail;
+		i += e_node->len;
 	}
-
 	/* read the tail-end data */
 	inode->idata_size = inode->i_size % EROFS_BLKSIZ;
 	if (inode->idata_size) {
@@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct erofs_buffer_head *bh)
 		if (ret)
 			return false;
 		free(inode->compressmeta);
+		off += inode->extent_isize;
 	}
 
+	if (inode->sparse_extent_isize) {
+		ret = erofs_write_sparse_extents(inode, off);
+		if (ret)
+			return false;
+	}
 	inode->bh = NULL;
 	erofs_iput(inode);
 	return erofs_bh_flush_generic_end(bh);
@@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)
 
 	init_list_head(&inode->i_subdirs);
 	init_list_head(&inode->i_xattrs);
+	init_list_head(&inode->i_sparse_extents);
 
 	inode->idata_size = 0;
 	inode->xattr_isize = 0;
-	inode->extent_isize = 0;
+	inode->sparse_extent_isize = 0;
 
 	inode->bh = inode->bh_inline = inode->bh_data = NULL;
 	inode->idata = NULL;
@@ -961,4 +1071,3 @@ struct erofs_inode *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
 
 	return erofs_mkfs_build_tree(inode);
 }
-
-- 
2.9.3


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

* Re: [RFCv3] erofs-utils: on-disk extent format for blocks
  2020-01-02  9:47 [RFCv3] erofs-utils: on-disk extent format for blocks Pratik Shinde
@ 2020-01-02 10:47 ` Gao Xiang
  2020-01-02 11:02   ` Pratik Shinde
  0 siblings, 1 reply; 6+ messages in thread
From: Gao Xiang @ 2020-01-02 10:47 UTC (permalink / raw)
  To: Pratik Shinde, Chao Yu, Li Guifu; +Cc: xiaoxie, linux-erofs

Hi Pratik,

On Thu, Jan 02, 2020 at 03:17:32PM +0530, Pratik Shinde wrote:
> 1)Moved on-disk structures to erofs_fs.h
> 2)Some naming changes.
> 
> I think we can keep 'IS_HOLE()' macro, otherwise the code
> does not look clean(if used directly/without macro).Its getting
> used only in inode.c so it can be kept there.
> what do you think ?

What I'm little concerned is the relationship between
the name of IS_HOLE and its implementation...

In other words, why
 roundup(start, EROFS_BLKSIZ) == start &&
 roundup(end, EROFS_BLKSIZ) == end &&
 (end - start) % EROFS_BLKSIZ == 0
should be an erofs hole...

But that is minor, I reserve my opinion on this for now...

This patch generally looks good to me, yet I haven't
played with it till now.

Could you implement a workable RFC patch for kernel side
as well? Since I'm still busying in XZ library and other
convert patches for 5.6...

I'd like to hear if some other opinions from Chao and Guifu.
Since it enhances the on-disk format, we need to think it
over (especially its future expandability).


To Chao and Guifu,
Could you have some extra time looking at this stuff as well?


Thanks,
Gao Xiang

> 
> Signed-off-by: Pratik Shinde <pratikshinde320@gmail.com>
> ---
>  include/erofs/internal.h |   9 ++-
>  include/erofs_fs.h       |  11 ++++
>  lib/inode.c              | 153 ++++++++++++++++++++++++++++++++++++++++-------
>  3 files changed, 150 insertions(+), 23 deletions(-)
> 
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index e13adda..2d7466b 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -63,7 +63,7 @@ struct erofs_sb_info {
>  extern struct erofs_sb_info sbi;
>  
>  struct erofs_inode {
> -	struct list_head i_hash, i_subdirs, i_xattrs;
> +	struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;
>  
>  	unsigned int i_count;
>  	struct erofs_inode *i_parent;
> @@ -93,6 +93,7 @@ struct erofs_inode {
>  
>  	unsigned int xattr_isize;
>  	unsigned int extent_isize;
> +	unsigned int sparse_extent_isize;
>  
>  	erofs_nid_t nid;
>  	struct erofs_buffer_head *bh;
> @@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int err)
>  	return msg;
>  }
>  
> +struct erofs_sparse_extent_node {
> +	struct list_head next;
> +	erofs_blk_t lblk;
> +	erofs_blk_t pblk;
> +	u32 len;
> +};
>  #endif
>  
> diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> index bcc4f0c..a63e1c6 100644
> --- a/include/erofs_fs.h
> +++ b/include/erofs_fs.h
> @@ -321,5 +321,16 @@ static inline void erofs_check_ondisk_layout_definitions(void)
>  		     Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
>  }
>  
> +/* on-disk sparse extent format */
> +struct erofs_sparse_extent {
> +	__le32 ee_lblk;
> +	__le32 ee_pblk;
> +	__le32 ee_len;
> +};
> +
> +struct erofs_sparse_extent_iheader {
> +	u32 count;
> +};
> +
>  #endif
>  
> diff --git a/lib/inode.c b/lib/inode.c
> index 0e19b11..da20599 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT >> S_SHIFT] = {
>  
>  struct list_head inode_hashtable[NR_INODE_HASHTABLE];
>  
> +
> +#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start &&	\
> +		             roundup(end, EROFS_BLKSIZ) == end &&	\
> +			    (end - start) % EROFS_BLKSIZ == 0)
> +
> +/**
> +   read extents of the given file.
> +   record the data extents and link them into a chain.
> +   exclude holes present in file.
> + */
> +unsigned int erofs_read_sparse_extents(int fd, struct list_head *extents)
> +{
> +	erofs_blk_t startblk, endblk, datablk;
> +	unsigned int nholes = 0;
> +	erofs_off_t data, hole, len = 0, last_data;
> +	struct erofs_sparse_extent_node *e_data;
> +
> +	len = lseek(fd, 0, SEEK_END);
> +	if (len < 0)
> +		return -errno;
> +	if (lseek(fd, 0, SEEK_SET) == -1)
> +		return -errno;
> +	data = 0;
> +	last_data = 0;
> +	while (data < len) {
> +		hole = lseek(fd, data, SEEK_HOLE);
> +		if (hole == len)
> +			break;
> +		data = lseek(fd, hole, SEEK_DATA);
> +		if (data < 0 || hole > data)
> +			return -EINVAL;
> +		if (IS_HOLE(hole, data)) {
> +			startblk = erofs_blknr(hole);
> +			datablk = erofs_blknr(last_data);
> +			endblk = erofs_blknr(data);
> +			last_data = data;
> +			e_data = malloc(sizeof(
> +					 struct erofs_sparse_extent_node));
> +			if (e_data == NULL)
> +				return -ENOMEM;
> +			e_data->lblk = datablk;
> +			e_data->len = (startblk - datablk);
> +			list_add_tail(&e_data->next, extents);
> +			nholes += (endblk - startblk);
> +		}
> +	}
> +	/* rounddown to exclude tail-end data */
> +	if (last_data < len && (len - last_data) >= EROFS_BLKSIZ) {
> +		e_data = malloc(sizeof(struct erofs_sparse_extent_node));
> +		if (e_data == NULL)
> +			return -ENOMEM;
> +		startblk = erofs_blknr(last_data);
> +		e_data->lblk = startblk;
> +		e_data->len = erofs_blknr(rounddown((len - last_data),
> +					  EROFS_BLKSIZ));
> +		list_add_tail(&e_data->next, extents);
> +	}
> +	return nholes;
> +}
> +
> +int erofs_write_sparse_extents(struct erofs_inode *inode, erofs_off_t off)
> +{
> +	struct erofs_sparse_extent_node *e_node;
> +	struct erofs_sparse_extent_iheader *header;
> +	char *buf;
> +	unsigned int p = 0;
> +	int ret;
> +
> +	buf = malloc(inode->sparse_extent_isize);
> +	if (buf == NULL)
> +		return -ENOMEM;
> +	header = (struct erofs_sparse_extent_iheader *) buf;
> +	header->count = 0;
> +	p += sizeof(struct erofs_sparse_extent_iheader);
> +	list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> +		const struct erofs_sparse_extent ee = {
> +			.ee_lblk = cpu_to_le32(e_node->lblk),
> +			.ee_pblk = cpu_to_le32(e_node->pblk),
> +			.ee_len  = cpu_to_le32(e_node->len)
> +		};
> +		memcpy(buf + p, &ee, sizeof(struct erofs_sparse_extent));
> +		p += sizeof(struct erofs_sparse_extent);
> +		header->count++;
> +		list_del(&e_node->next);
> +		free(e_node);
> +	}
> +	ret = dev_write(buf, off, inode->sparse_extent_isize);
> +	free(buf);
> +	return ret;
> +}
> +
>  void erofs_inode_manager_init(void)
>  {
>  	unsigned int i;
> @@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct erofs_inode *inode)
>  
>  int erofs_write_file(struct erofs_inode *inode)
>  {
> -	unsigned int nblocks, i;
> +	unsigned int nblocks, i, j, nholes;
>  	int ret, fd;
> +	struct erofs_sparse_extent_node *e_node;
>  
>  	if (!inode->i_size) {
>  		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> @@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)
>  	/* fallback to all data uncompressed */
>  	inode->datalayout = EROFS_INODE_FLAT_INLINE;
>  	nblocks = inode->i_size / EROFS_BLKSIZ;
> -
> -	ret = __allocate_inode_bh_data(inode, nblocks);
> -	if (ret)
> -		return ret;
> -
>  	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
>  	if (fd < 0)
>  		return -errno;
> -
> -	for (i = 0; i < nblocks; ++i) {
> -		char buf[EROFS_BLKSIZ];
> -
> -		ret = read(fd, buf, EROFS_BLKSIZ);
> -		if (ret != EROFS_BLKSIZ) {
> -			if (ret < 0)
> +	nholes = erofs_read_sparse_extents(fd, &inode->i_sparse_extents);
> +	if (nholes < 0) {
> +		close(fd);
> +		return nholes;
> +	}
> +	ret = __allocate_inode_bh_data(inode, nblocks - nholes);
> +	if (ret) {
> +		close(fd);
> +		return ret;
> +	}
> +	i = inode->u.i_blkaddr;
> +	inode->sparse_extent_isize = sizeof(struct erofs_sparse_extent_iheader);
> +	list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> +		inode->sparse_extent_isize += sizeof(struct erofs_sparse_extent);
> +		e_node->pblk = i;
> +		ret = lseek(fd, blknr_to_addr(e_node->lblk), SEEK_SET);
> +		if (ret < 0)
> +			goto fail;
> +		for (j = 0; j < e_node->len; j++) {
> +			char buf[EROFS_BLKSIZ];
> +			ret = read(fd, buf, EROFS_BLKSIZ);
> +			if (ret != EROFS_BLKSIZ) {
> +				if (ret < 0)
> +					goto fail;
> +				close(fd);
> +				return -EAGAIN;
> +			}
> +			ret = blk_write(buf, e_node->pblk + j, 1);
> +			if (ret)
>  				goto fail;
> -			close(fd);
> -			return -EAGAIN;
>  		}
> -
> -		ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
> -		if (ret)
> -			goto fail;
> +		i += e_node->len;
>  	}
> -
>  	/* read the tail-end data */
>  	inode->idata_size = inode->i_size % EROFS_BLKSIZ;
>  	if (inode->idata_size) {
> @@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct erofs_buffer_head *bh)
>  		if (ret)
>  			return false;
>  		free(inode->compressmeta);
> +		off += inode->extent_isize;
>  	}
>  
> +	if (inode->sparse_extent_isize) {
> +		ret = erofs_write_sparse_extents(inode, off);
> +		if (ret)
> +			return false;
> +	}
>  	inode->bh = NULL;
>  	erofs_iput(inode);
>  	return erofs_bh_flush_generic_end(bh);
> @@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)
>  
>  	init_list_head(&inode->i_subdirs);
>  	init_list_head(&inode->i_xattrs);
> +	init_list_head(&inode->i_sparse_extents);
>  
>  	inode->idata_size = 0;
>  	inode->xattr_isize = 0;
> -	inode->extent_isize = 0;
> +	inode->sparse_extent_isize = 0;
>  
>  	inode->bh = inode->bh_inline = inode->bh_data = NULL;
>  	inode->idata = NULL;
> @@ -961,4 +1071,3 @@ struct erofs_inode *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
>  
>  	return erofs_mkfs_build_tree(inode);
>  }
> -
> -- 
> 2.9.3
> 

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

* Re: [RFCv3] erofs-utils: on-disk extent format for blocks
  2020-01-02 10:47 ` Gao Xiang
@ 2020-01-02 11:02   ` Pratik Shinde
  2020-01-02 13:33     ` Gao Xiang
  0 siblings, 1 reply; 6+ messages in thread
From: Pratik Shinde @ 2020-01-02 11:02 UTC (permalink / raw)
  To: Gao Xiang; +Cc: xiaoxie, linux-erofs

[-- Attachment #1: Type: text/plain, Size: 11700 bytes --]

Hi Gao,

You are correct. The macro logic can be simplified. I will do that.
I will work on the kernel part of this change & do some testing on it.
I will keep you posted about the change and relevant tests I am running on
it.

--Pratik.











On Thu, Jan 2, 2020, 4:17 PM Gao Xiang <gaoxiang25@huawei.com> wrote:

> Hi Pratik,
>
> On Thu, Jan 02, 2020 at 03:17:32PM +0530, Pratik Shinde wrote:
> > 1)Moved on-disk structures to erofs_fs.h
> > 2)Some naming changes.
> >
> > I think we can keep 'IS_HOLE()' macro, otherwise the code
> > does not look clean(if used directly/without macro).Its getting
> > used only in inode.c so it can be kept there.
> > what do you think ?
>
> What I'm little concerned is the relationship between
> the name of IS_HOLE and its implementation...
>
> In other words, why
>  roundup(start, EROFS_BLKSIZ) == start &&
>  roundup(end, EROFS_BLKSIZ) == end &&
>  (end - start) % EROFS_BLKSIZ == 0
> should be an erofs hole...
>
> But that is minor, I reserve my opinion on this for now...
>
> This patch generally looks good to me, yet I haven't
> played with it till now.
>
> Could you implement a workable RFC patch for kernel side
> as well? Since I'm still busying in XZ library and other
> convert patches for 5.6...
>
> I'd like to hear if some other opinions from Chao and Guifu.
> Since it enhances the on-disk format, we need to think it
> over (especially its future expandability).
>
>
> To Chao and Guifu,
> Could you have some extra time looking at this stuff as well?
>
>
> Thanks,
> Gao Xiang
>
> >
> > Signed-off-by: Pratik Shinde <pratikshinde320@gmail.com>
> > ---
> >  include/erofs/internal.h |   9 ++-
> >  include/erofs_fs.h       |  11 ++++
> >  lib/inode.c              | 153
> ++++++++++++++++++++++++++++++++++++++++-------
> >  3 files changed, 150 insertions(+), 23 deletions(-)
> >
> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > index e13adda..2d7466b 100644
> > --- a/include/erofs/internal.h
> > +++ b/include/erofs/internal.h
> > @@ -63,7 +63,7 @@ struct erofs_sb_info {
> >  extern struct erofs_sb_info sbi;
> >
> >  struct erofs_inode {
> > -     struct list_head i_hash, i_subdirs, i_xattrs;
> > +     struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;
> >
> >       unsigned int i_count;
> >       struct erofs_inode *i_parent;
> > @@ -93,6 +93,7 @@ struct erofs_inode {
> >
> >       unsigned int xattr_isize;
> >       unsigned int extent_isize;
> > +     unsigned int sparse_extent_isize;
> >
> >       erofs_nid_t nid;
> >       struct erofs_buffer_head *bh;
> > @@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int err)
> >       return msg;
> >  }
> >
> > +struct erofs_sparse_extent_node {
> > +     struct list_head next;
> > +     erofs_blk_t lblk;
> > +     erofs_blk_t pblk;
> > +     u32 len;
> > +};
> >  #endif
> >
> > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > index bcc4f0c..a63e1c6 100644
> > --- a/include/erofs_fs.h
> > +++ b/include/erofs_fs.h
> > @@ -321,5 +321,16 @@ static inline void
> erofs_check_ondisk_layout_definitions(void)
> >                    Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> >  }
> >
> > +/* on-disk sparse extent format */
> > +struct erofs_sparse_extent {
> > +     __le32 ee_lblk;
> > +     __le32 ee_pblk;
> > +     __le32 ee_len;
> > +};
> > +
> > +struct erofs_sparse_extent_iheader {
> > +     u32 count;
> > +};
> > +
> >  #endif
> >
> > diff --git a/lib/inode.c b/lib/inode.c
> > index 0e19b11..da20599 100644
> > --- a/lib/inode.c
> > +++ b/lib/inode.c
> > @@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT >>
> S_SHIFT] = {
> >
> >  struct list_head inode_hashtable[NR_INODE_HASHTABLE];
> >
> > +
> > +#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start &&
>       \
> > +                          roundup(end, EROFS_BLKSIZ) == end &&       \
> > +                         (end - start) % EROFS_BLKSIZ == 0)
> > +
> > +/**
> > +   read extents of the given file.
> > +   record the data extents and link them into a chain.
> > +   exclude holes present in file.
> > + */
> > +unsigned int erofs_read_sparse_extents(int fd, struct list_head
> *extents)
> > +{
> > +     erofs_blk_t startblk, endblk, datablk;
> > +     unsigned int nholes = 0;
> > +     erofs_off_t data, hole, len = 0, last_data;
> > +     struct erofs_sparse_extent_node *e_data;
> > +
> > +     len = lseek(fd, 0, SEEK_END);
> > +     if (len < 0)
> > +             return -errno;
> > +     if (lseek(fd, 0, SEEK_SET) == -1)
> > +             return -errno;
> > +     data = 0;
> > +     last_data = 0;
> > +     while (data < len) {
> > +             hole = lseek(fd, data, SEEK_HOLE);
> > +             if (hole == len)
> > +                     break;
> > +             data = lseek(fd, hole, SEEK_DATA);
> > +             if (data < 0 || hole > data)
> > +                     return -EINVAL;
> > +             if (IS_HOLE(hole, data)) {
> > +                     startblk = erofs_blknr(hole);
> > +                     datablk = erofs_blknr(last_data);
> > +                     endblk = erofs_blknr(data);
> > +                     last_data = data;
> > +                     e_data = malloc(sizeof(
> > +                                      struct erofs_sparse_extent_node));
> > +                     if (e_data == NULL)
> > +                             return -ENOMEM;
> > +                     e_data->lblk = datablk;
> > +                     e_data->len = (startblk - datablk);
> > +                     list_add_tail(&e_data->next, extents);
> > +                     nholes += (endblk - startblk);
> > +             }
> > +     }
> > +     /* rounddown to exclude tail-end data */
> > +     if (last_data < len && (len - last_data) >= EROFS_BLKSIZ) {
> > +             e_data = malloc(sizeof(struct erofs_sparse_extent_node));
> > +             if (e_data == NULL)
> > +                     return -ENOMEM;
> > +             startblk = erofs_blknr(last_data);
> > +             e_data->lblk = startblk;
> > +             e_data->len = erofs_blknr(rounddown((len - last_data),
> > +                                       EROFS_BLKSIZ));
> > +             list_add_tail(&e_data->next, extents);
> > +     }
> > +     return nholes;
> > +}
> > +
> > +int erofs_write_sparse_extents(struct erofs_inode *inode, erofs_off_t
> off)
> > +{
> > +     struct erofs_sparse_extent_node *e_node;
> > +     struct erofs_sparse_extent_iheader *header;
> > +     char *buf;
> > +     unsigned int p = 0;
> > +     int ret;
> > +
> > +     buf = malloc(inode->sparse_extent_isize);
> > +     if (buf == NULL)
> > +             return -ENOMEM;
> > +     header = (struct erofs_sparse_extent_iheader *) buf;
> > +     header->count = 0;
> > +     p += sizeof(struct erofs_sparse_extent_iheader);
> > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > +             const struct erofs_sparse_extent ee = {
> > +                     .ee_lblk = cpu_to_le32(e_node->lblk),
> > +                     .ee_pblk = cpu_to_le32(e_node->pblk),
> > +                     .ee_len  = cpu_to_le32(e_node->len)
> > +             };
> > +             memcpy(buf + p, &ee, sizeof(struct erofs_sparse_extent));
> > +             p += sizeof(struct erofs_sparse_extent);
> > +             header->count++;
> > +             list_del(&e_node->next);
> > +             free(e_node);
> > +     }
> > +     ret = dev_write(buf, off, inode->sparse_extent_isize);
> > +     free(buf);
> > +     return ret;
> > +}
> > +
> >  void erofs_inode_manager_init(void)
> >  {
> >       unsigned int i;
> > @@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct
> erofs_inode *inode)
> >
> >  int erofs_write_file(struct erofs_inode *inode)
> >  {
> > -     unsigned int nblocks, i;
> > +     unsigned int nblocks, i, j, nholes;
> >       int ret, fd;
> > +     struct erofs_sparse_extent_node *e_node;
> >
> >       if (!inode->i_size) {
> >               inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > @@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)
> >       /* fallback to all data uncompressed */
> >       inode->datalayout = EROFS_INODE_FLAT_INLINE;
> >       nblocks = inode->i_size / EROFS_BLKSIZ;
> > -
> > -     ret = __allocate_inode_bh_data(inode, nblocks);
> > -     if (ret)
> > -             return ret;
> > -
> >       fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> >       if (fd < 0)
> >               return -errno;
> > -
> > -     for (i = 0; i < nblocks; ++i) {
> > -             char buf[EROFS_BLKSIZ];
> > -
> > -             ret = read(fd, buf, EROFS_BLKSIZ);
> > -             if (ret != EROFS_BLKSIZ) {
> > -                     if (ret < 0)
> > +     nholes = erofs_read_sparse_extents(fd, &inode->i_sparse_extents);
> > +     if (nholes < 0) {
> > +             close(fd);
> > +             return nholes;
> > +     }
> > +     ret = __allocate_inode_bh_data(inode, nblocks - nholes);
> > +     if (ret) {
> > +             close(fd);
> > +             return ret;
> > +     }
> > +     i = inode->u.i_blkaddr;
> > +     inode->sparse_extent_isize = sizeof(struct
> erofs_sparse_extent_iheader);
> > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > +             inode->sparse_extent_isize += sizeof(struct
> erofs_sparse_extent);
> > +             e_node->pblk = i;
> > +             ret = lseek(fd, blknr_to_addr(e_node->lblk), SEEK_SET);
> > +             if (ret < 0)
> > +                     goto fail;
> > +             for (j = 0; j < e_node->len; j++) {
> > +                     char buf[EROFS_BLKSIZ];
> > +                     ret = read(fd, buf, EROFS_BLKSIZ);
> > +                     if (ret != EROFS_BLKSIZ) {
> > +                             if (ret < 0)
> > +                                     goto fail;
> > +                             close(fd);
> > +                             return -EAGAIN;
> > +                     }
> > +                     ret = blk_write(buf, e_node->pblk + j, 1);
> > +                     if (ret)
> >                               goto fail;
> > -                     close(fd);
> > -                     return -EAGAIN;
> >               }
> > -
> > -             ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
> > -             if (ret)
> > -                     goto fail;
> > +             i += e_node->len;
> >       }
> > -
> >       /* read the tail-end data */
> >       inode->idata_size = inode->i_size % EROFS_BLKSIZ;
> >       if (inode->idata_size) {
> > @@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct
> erofs_buffer_head *bh)
> >               if (ret)
> >                       return false;
> >               free(inode->compressmeta);
> > +             off += inode->extent_isize;
> >       }
> >
> > +     if (inode->sparse_extent_isize) {
> > +             ret = erofs_write_sparse_extents(inode, off);
> > +             if (ret)
> > +                     return false;
> > +     }
> >       inode->bh = NULL;
> >       erofs_iput(inode);
> >       return erofs_bh_flush_generic_end(bh);
> > @@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)
> >
> >       init_list_head(&inode->i_subdirs);
> >       init_list_head(&inode->i_xattrs);
> > +     init_list_head(&inode->i_sparse_extents);
> >
> >       inode->idata_size = 0;
> >       inode->xattr_isize = 0;
> > -     inode->extent_isize = 0;
> > +     inode->sparse_extent_isize = 0;
> >
> >       inode->bh = inode->bh_inline = inode->bh_data = NULL;
> >       inode->idata = NULL;
> > @@ -961,4 +1071,3 @@ struct erofs_inode
> *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
> >
> >       return erofs_mkfs_build_tree(inode);
> >  }
> > -
> > --
> > 2.9.3
> >
>

[-- Attachment #2: Type: text/html, Size: 15728 bytes --]

<div dir="auto">Hi Gao,<div dir="auto"><br></div><div dir="auto">You are correct. The macro logic can be simplified. I will do that.</div><div dir="auto">I will work on the kernel part of this change &amp; do some testing on it.</div><div dir="auto">I will keep you posted about the change and relevant tests I am running on it.</div><div dir="auto"><br></div><div dir="auto">--Pratik.</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 2, 2020, 4:17 PM Gao Xiang &lt;<a href="mailto:gaoxiang25@huawei.com">gaoxiang25@huawei.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Pratik,<br>
<br>
On Thu, Jan 02, 2020 at 03:17:32PM +0530, Pratik Shinde wrote:<br>
&gt; 1)Moved on-disk structures to erofs_fs.h<br>
&gt; 2)Some naming changes.<br>
&gt; <br>
&gt; I think we can keep &#39;IS_HOLE()&#39; macro, otherwise the code<br>
&gt; does not look clean(if used directly/without macro).Its getting<br>
&gt; used only in inode.c so it can be kept there.<br>
&gt; what do you think ?<br>
<br>
What I&#39;m little concerned is the relationship between<br>
the name of IS_HOLE and its implementation...<br>
<br>
In other words, why<br>
 roundup(start, EROFS_BLKSIZ) == start &amp;&amp;<br>
 roundup(end, EROFS_BLKSIZ) == end &amp;&amp;<br>
 (end - start) % EROFS_BLKSIZ == 0<br>
should be an erofs hole...<br>
<br>
But that is minor, I reserve my opinion on this for now...<br>
<br>
This patch generally looks good to me, yet I haven&#39;t<br>
played with it till now.<br>
<br>
Could you implement a workable RFC patch for kernel side<br>
as well? Since I&#39;m still busying in XZ library and other<br>
convert patches for 5.6...<br>
<br>
I&#39;d like to hear if some other opinions from Chao and Guifu.<br>
Since it enhances the on-disk format, we need to think it<br>
over (especially its future expandability).<br>
<br>
<br>
To Chao and Guifu,<br>
Could you have some extra time looking at this stuff as well?<br>
<br>
<br>
Thanks,<br>
Gao Xiang<br>
<br>
&gt; <br>
&gt; Signed-off-by: Pratik Shinde &lt;<a href="mailto:pratikshinde320@gmail.com" target="_blank" rel="noreferrer">pratikshinde320@gmail.com</a>&gt;<br>
&gt; ---<br>
&gt;  include/erofs/internal.h |   9 ++-<br>
&gt;  include/erofs_fs.h       |  11 ++++<br>
&gt;  lib/inode.c              | 153 ++++++++++++++++++++++++++++++++++++++++-------<br>
&gt;  3 files changed, 150 insertions(+), 23 deletions(-)<br>
&gt; <br>
&gt; diff --git a/include/erofs/internal.h b/include/erofs/internal.h<br>
&gt; index e13adda..2d7466b 100644<br>
&gt; --- a/include/erofs/internal.h<br>
&gt; +++ b/include/erofs/internal.h<br>
&gt; @@ -63,7 +63,7 @@ struct erofs_sb_info {<br>
&gt;  extern struct erofs_sb_info sbi;<br>
&gt;  <br>
&gt;  struct erofs_inode {<br>
&gt; -     struct list_head i_hash, i_subdirs, i_xattrs;<br>
&gt; +     struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;<br>
&gt;  <br>
&gt;       unsigned int i_count;<br>
&gt;       struct erofs_inode *i_parent;<br>
&gt; @@ -93,6 +93,7 @@ struct erofs_inode {<br>
&gt;  <br>
&gt;       unsigned int xattr_isize;<br>
&gt;       unsigned int extent_isize;<br>
&gt; +     unsigned int sparse_extent_isize;<br>
&gt;  <br>
&gt;       erofs_nid_t nid;<br>
&gt;       struct erofs_buffer_head *bh;<br>
&gt; @@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int err)<br>
&gt;       return msg;<br>
&gt;  }<br>
&gt;  <br>
&gt; +struct erofs_sparse_extent_node {<br>
&gt; +     struct list_head next;<br>
&gt; +     erofs_blk_t lblk;<br>
&gt; +     erofs_blk_t pblk;<br>
&gt; +     u32 len;<br>
&gt; +};<br>
&gt;  #endif<br>
&gt;  <br>
&gt; diff --git a/include/erofs_fs.h b/include/erofs_fs.h<br>
&gt; index bcc4f0c..a63e1c6 100644<br>
&gt; --- a/include/erofs_fs.h<br>
&gt; +++ b/include/erofs_fs.h<br>
&gt; @@ -321,5 +321,16 @@ static inline void erofs_check_ondisk_layout_definitions(void)<br>
&gt;                    Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);<br>
&gt;  }<br>
&gt;  <br>
&gt; +/* on-disk sparse extent format */<br>
&gt; +struct erofs_sparse_extent {<br>
&gt; +     __le32 ee_lblk;<br>
&gt; +     __le32 ee_pblk;<br>
&gt; +     __le32 ee_len;<br>
&gt; +};<br>
&gt; +<br>
&gt; +struct erofs_sparse_extent_iheader {<br>
&gt; +     u32 count;<br>
&gt; +};<br>
&gt; +<br>
&gt;  #endif<br>
&gt;  <br>
&gt; diff --git a/lib/inode.c b/lib/inode.c<br>
&gt; index 0e19b11..da20599 100644<br>
&gt; --- a/lib/inode.c<br>
&gt; +++ b/lib/inode.c<br>
&gt; @@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT &gt;&gt; S_SHIFT] = {<br>
&gt;  <br>
&gt;  struct list_head inode_hashtable[NR_INODE_HASHTABLE];<br>
&gt;  <br>
&gt; +<br>
&gt; +#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start &amp;&amp;        \<br>
&gt; +                          roundup(end, EROFS_BLKSIZ) == end &amp;&amp;       \<br>
&gt; +                         (end - start) % EROFS_BLKSIZ == 0)<br>
&gt; +<br>
&gt; +/**<br>
&gt; +   read extents of the given file.<br>
&gt; +   record the data extents and link them into a chain.<br>
&gt; +   exclude holes present in file.<br>
&gt; + */<br>
&gt; +unsigned int erofs_read_sparse_extents(int fd, struct list_head *extents)<br>
&gt; +{<br>
&gt; +     erofs_blk_t startblk, endblk, datablk;<br>
&gt; +     unsigned int nholes = 0;<br>
&gt; +     erofs_off_t data, hole, len = 0, last_data;<br>
&gt; +     struct erofs_sparse_extent_node *e_data;<br>
&gt; +<br>
&gt; +     len = lseek(fd, 0, SEEK_END);<br>
&gt; +     if (len &lt; 0)<br>
&gt; +             return -errno;<br>
&gt; +     if (lseek(fd, 0, SEEK_SET) == -1)<br>
&gt; +             return -errno;<br>
&gt; +     data = 0;<br>
&gt; +     last_data = 0;<br>
&gt; +     while (data &lt; len) {<br>
&gt; +             hole = lseek(fd, data, SEEK_HOLE);<br>
&gt; +             if (hole == len)<br>
&gt; +                     break;<br>
&gt; +             data = lseek(fd, hole, SEEK_DATA);<br>
&gt; +             if (data &lt; 0 || hole &gt; data)<br>
&gt; +                     return -EINVAL;<br>
&gt; +             if (IS_HOLE(hole, data)) {<br>
&gt; +                     startblk = erofs_blknr(hole);<br>
&gt; +                     datablk = erofs_blknr(last_data);<br>
&gt; +                     endblk = erofs_blknr(data);<br>
&gt; +                     last_data = data;<br>
&gt; +                     e_data = malloc(sizeof(<br>
&gt; +                                      struct erofs_sparse_extent_node));<br>
&gt; +                     if (e_data == NULL)<br>
&gt; +                             return -ENOMEM;<br>
&gt; +                     e_data-&gt;lblk = datablk;<br>
&gt; +                     e_data-&gt;len = (startblk - datablk);<br>
&gt; +                     list_add_tail(&amp;e_data-&gt;next, extents);<br>
&gt; +                     nholes += (endblk - startblk);<br>
&gt; +             }<br>
&gt; +     }<br>
&gt; +     /* rounddown to exclude tail-end data */<br>
&gt; +     if (last_data &lt; len &amp;&amp; (len - last_data) &gt;= EROFS_BLKSIZ) {<br>
&gt; +             e_data = malloc(sizeof(struct erofs_sparse_extent_node));<br>
&gt; +             if (e_data == NULL)<br>
&gt; +                     return -ENOMEM;<br>
&gt; +             startblk = erofs_blknr(last_data);<br>
&gt; +             e_data-&gt;lblk = startblk;<br>
&gt; +             e_data-&gt;len = erofs_blknr(rounddown((len - last_data),<br>
&gt; +                                       EROFS_BLKSIZ));<br>
&gt; +             list_add_tail(&amp;e_data-&gt;next, extents);<br>
&gt; +     }<br>
&gt; +     return nholes;<br>
&gt; +}<br>
&gt; +<br>
&gt; +int erofs_write_sparse_extents(struct erofs_inode *inode, erofs_off_t off)<br>
&gt; +{<br>
&gt; +     struct erofs_sparse_extent_node *e_node;<br>
&gt; +     struct erofs_sparse_extent_iheader *header;<br>
&gt; +     char *buf;<br>
&gt; +     unsigned int p = 0;<br>
&gt; +     int ret;<br>
&gt; +<br>
&gt; +     buf = malloc(inode-&gt;sparse_extent_isize);<br>
&gt; +     if (buf == NULL)<br>
&gt; +             return -ENOMEM;<br>
&gt; +     header = (struct erofs_sparse_extent_iheader *) buf;<br>
&gt; +     header-&gt;count = 0;<br>
&gt; +     p += sizeof(struct erofs_sparse_extent_iheader);<br>
&gt; +     list_for_each_entry(e_node, &amp;inode-&gt;i_sparse_extents, next) {<br>
&gt; +             const struct erofs_sparse_extent ee = {<br>
&gt; +                     .ee_lblk = cpu_to_le32(e_node-&gt;lblk),<br>
&gt; +                     .ee_pblk = cpu_to_le32(e_node-&gt;pblk),<br>
&gt; +                     .ee_len  = cpu_to_le32(e_node-&gt;len)<br>
&gt; +             };<br>
&gt; +             memcpy(buf + p, &amp;ee, sizeof(struct erofs_sparse_extent));<br>
&gt; +             p += sizeof(struct erofs_sparse_extent);<br>
&gt; +             header-&gt;count++;<br>
&gt; +             list_del(&amp;e_node-&gt;next);<br>
&gt; +             free(e_node);<br>
&gt; +     }<br>
&gt; +     ret = dev_write(buf, off, inode-&gt;sparse_extent_isize);<br>
&gt; +     free(buf);<br>
&gt; +     return ret;<br>
&gt; +}<br>
&gt; +<br>
&gt;  void erofs_inode_manager_init(void)<br>
&gt;  {<br>
&gt;       unsigned int i;<br>
&gt; @@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct erofs_inode *inode)<br>
&gt;  <br>
&gt;  int erofs_write_file(struct erofs_inode *inode)<br>
&gt;  {<br>
&gt; -     unsigned int nblocks, i;<br>
&gt; +     unsigned int nblocks, i, j, nholes;<br>
&gt;       int ret, fd;<br>
&gt; +     struct erofs_sparse_extent_node *e_node;<br>
&gt;  <br>
&gt;       if (!inode-&gt;i_size) {<br>
&gt;               inode-&gt;datalayout = EROFS_INODE_FLAT_PLAIN;<br>
&gt; @@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)<br>
&gt;       /* fallback to all data uncompressed */<br>
&gt;       inode-&gt;datalayout = EROFS_INODE_FLAT_INLINE;<br>
&gt;       nblocks = inode-&gt;i_size / EROFS_BLKSIZ;<br>
&gt; -<br>
&gt; -     ret = __allocate_inode_bh_data(inode, nblocks);<br>
&gt; -     if (ret)<br>
&gt; -             return ret;<br>
&gt; -<br>
&gt;       fd = open(inode-&gt;i_srcpath, O_RDONLY | O_BINARY);<br>
&gt;       if (fd &lt; 0)<br>
&gt;               return -errno;<br>
&gt; -<br>
&gt; -     for (i = 0; i &lt; nblocks; ++i) {<br>
&gt; -             char buf[EROFS_BLKSIZ];<br>
&gt; -<br>
&gt; -             ret = read(fd, buf, EROFS_BLKSIZ);<br>
&gt; -             if (ret != EROFS_BLKSIZ) {<br>
&gt; -                     if (ret &lt; 0)<br>
&gt; +     nholes = erofs_read_sparse_extents(fd, &amp;inode-&gt;i_sparse_extents);<br>
&gt; +     if (nholes &lt; 0) {<br>
&gt; +             close(fd);<br>
&gt; +             return nholes;<br>
&gt; +     }<br>
&gt; +     ret = __allocate_inode_bh_data(inode, nblocks - nholes);<br>
&gt; +     if (ret) {<br>
&gt; +             close(fd);<br>
&gt; +             return ret;<br>
&gt; +     }<br>
&gt; +     i = inode-&gt;u.i_blkaddr;<br>
&gt; +     inode-&gt;sparse_extent_isize = sizeof(struct erofs_sparse_extent_iheader);<br>
&gt; +     list_for_each_entry(e_node, &amp;inode-&gt;i_sparse_extents, next) {<br>
&gt; +             inode-&gt;sparse_extent_isize += sizeof(struct erofs_sparse_extent);<br>
&gt; +             e_node-&gt;pblk = i;<br>
&gt; +             ret = lseek(fd, blknr_to_addr(e_node-&gt;lblk), SEEK_SET);<br>
&gt; +             if (ret &lt; 0)<br>
&gt; +                     goto fail;<br>
&gt; +             for (j = 0; j &lt; e_node-&gt;len; j++) {<br>
&gt; +                     char buf[EROFS_BLKSIZ];<br>
&gt; +                     ret = read(fd, buf, EROFS_BLKSIZ);<br>
&gt; +                     if (ret != EROFS_BLKSIZ) {<br>
&gt; +                             if (ret &lt; 0)<br>
&gt; +                                     goto fail;<br>
&gt; +                             close(fd);<br>
&gt; +                             return -EAGAIN;<br>
&gt; +                     }<br>
&gt; +                     ret = blk_write(buf, e_node-&gt;pblk + j, 1);<br>
&gt; +                     if (ret)<br>
&gt;                               goto fail;<br>
&gt; -                     close(fd);<br>
&gt; -                     return -EAGAIN;<br>
&gt;               }<br>
&gt; -<br>
&gt; -             ret = blk_write(buf, inode-&gt;u.i_blkaddr + i, 1);<br>
&gt; -             if (ret)<br>
&gt; -                     goto fail;<br>
&gt; +             i += e_node-&gt;len;<br>
&gt;       }<br>
&gt; -<br>
&gt;       /* read the tail-end data */<br>
&gt;       inode-&gt;idata_size = inode-&gt;i_size % EROFS_BLKSIZ;<br>
&gt;       if (inode-&gt;idata_size) {<br>
&gt; @@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct erofs_buffer_head *bh)<br>
&gt;               if (ret)<br>
&gt;                       return false;<br>
&gt;               free(inode-&gt;compressmeta);<br>
&gt; +             off += inode-&gt;extent_isize;<br>
&gt;       }<br>
&gt;  <br>
&gt; +     if (inode-&gt;sparse_extent_isize) {<br>
&gt; +             ret = erofs_write_sparse_extents(inode, off);<br>
&gt; +             if (ret)<br>
&gt; +                     return false;<br>
&gt; +     }<br>
&gt;       inode-&gt;bh = NULL;<br>
&gt;       erofs_iput(inode);<br>
&gt;       return erofs_bh_flush_generic_end(bh);<br>
&gt; @@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)<br>
&gt;  <br>
&gt;       init_list_head(&amp;inode-&gt;i_subdirs);<br>
&gt;       init_list_head(&amp;inode-&gt;i_xattrs);<br>
&gt; +     init_list_head(&amp;inode-&gt;i_sparse_extents);<br>
&gt;  <br>
&gt;       inode-&gt;idata_size = 0;<br>
&gt;       inode-&gt;xattr_isize = 0;<br>
&gt; -     inode-&gt;extent_isize = 0;<br>
&gt; +     inode-&gt;sparse_extent_isize = 0;<br>
&gt;  <br>
&gt;       inode-&gt;bh = inode-&gt;bh_inline = inode-&gt;bh_data = NULL;<br>
&gt;       inode-&gt;idata = NULL;<br>
&gt; @@ -961,4 +1071,3 @@ struct erofs_inode *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,<br>
&gt;  <br>
&gt;       return erofs_mkfs_build_tree(inode);<br>
&gt;  }<br>
&gt; -<br>
&gt; -- <br>
&gt; 2.9.3<br>
&gt; <br>
</blockquote></div>

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

* Re: [RFCv3] erofs-utils: on-disk extent format for blocks
  2020-01-02 11:02   ` Pratik Shinde
@ 2020-01-02 13:33     ` Gao Xiang
  2020-01-28  6:01       ` Pratik Shinde
  0 siblings, 1 reply; 6+ messages in thread
From: Gao Xiang @ 2020-01-02 13:33 UTC (permalink / raw)
  To: Pratik Shinde; +Cc: miaoxie, linux-erofs

On Thu, Jan 02, 2020 at 04:32:34PM +0530, Pratik Shinde wrote:
> Hi Gao,
> 
> You are correct. The macro logic can be simplified. I will do that.
> I will work on the kernel part of this change & do some testing on it.
> I will keep you posted about the change and relevant tests I am running on
> it.
> 
> --Pratik.
>

Okay, thanks for your effort. :)

Thanks,
Gao Xiang

> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> On Thu, Jan 2, 2020, 4:17 PM Gao Xiang <gaoxiang25@huawei.com> wrote:
> 
> > Hi Pratik,
> >
> > On Thu, Jan 02, 2020 at 03:17:32PM +0530, Pratik Shinde wrote:
> > > 1)Moved on-disk structures to erofs_fs.h
> > > 2)Some naming changes.
> > >
> > > I think we can keep 'IS_HOLE()' macro, otherwise the code
> > > does not look clean(if used directly/without macro).Its getting
> > > used only in inode.c so it can be kept there.
> > > what do you think ?
> >
> > What I'm little concerned is the relationship between
> > the name of IS_HOLE and its implementation...
> >
> > In other words, why
> >  roundup(start, EROFS_BLKSIZ) == start &&
> >  roundup(end, EROFS_BLKSIZ) == end &&
> >  (end - start) % EROFS_BLKSIZ == 0
> > should be an erofs hole...
> >
> > But that is minor, I reserve my opinion on this for now...
> >
> > This patch generally looks good to me, yet I haven't
> > played with it till now.
> >
> > Could you implement a workable RFC patch for kernel side
> > as well? Since I'm still busying in XZ library and other
> > convert patches for 5.6...
> >
> > I'd like to hear if some other opinions from Chao and Guifu.
> > Since it enhances the on-disk format, we need to think it
> > over (especially its future expandability).
> >
> >
> > To Chao and Guifu,
> > Could you have some extra time looking at this stuff as well?
> >
> >
> > Thanks,
> > Gao Xiang
> >
> > >
> > > Signed-off-by: Pratik Shinde <pratikshinde320@gmail.com>
> > > ---
> > >  include/erofs/internal.h |   9 ++-
> > >  include/erofs_fs.h       |  11 ++++
> > >  lib/inode.c              | 153
> > ++++++++++++++++++++++++++++++++++++++++-------
> > >  3 files changed, 150 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > index e13adda..2d7466b 100644
> > > --- a/include/erofs/internal.h
> > > +++ b/include/erofs/internal.h
> > > @@ -63,7 +63,7 @@ struct erofs_sb_info {
> > >  extern struct erofs_sb_info sbi;
> > >
> > >  struct erofs_inode {
> > > -     struct list_head i_hash, i_subdirs, i_xattrs;
> > > +     struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;
> > >
> > >       unsigned int i_count;
> > >       struct erofs_inode *i_parent;
> > > @@ -93,6 +93,7 @@ struct erofs_inode {
> > >
> > >       unsigned int xattr_isize;
> > >       unsigned int extent_isize;
> > > +     unsigned int sparse_extent_isize;
> > >
> > >       erofs_nid_t nid;
> > >       struct erofs_buffer_head *bh;
> > > @@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int err)
> > >       return msg;
> > >  }
> > >
> > > +struct erofs_sparse_extent_node {
> > > +     struct list_head next;
> > > +     erofs_blk_t lblk;
> > > +     erofs_blk_t pblk;
> > > +     u32 len;
> > > +};
> > >  #endif
> > >
> > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > index bcc4f0c..a63e1c6 100644
> > > --- a/include/erofs_fs.h
> > > +++ b/include/erofs_fs.h
> > > @@ -321,5 +321,16 @@ static inline void
> > erofs_check_ondisk_layout_definitions(void)
> > >                    Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > >  }
> > >
> > > +/* on-disk sparse extent format */
> > > +struct erofs_sparse_extent {
> > > +     __le32 ee_lblk;
> > > +     __le32 ee_pblk;
> > > +     __le32 ee_len;
> > > +};
> > > +
> > > +struct erofs_sparse_extent_iheader {
> > > +     u32 count;
> > > +};
> > > +
> > >  #endif
> > >
> > > diff --git a/lib/inode.c b/lib/inode.c
> > > index 0e19b11..da20599 100644
> > > --- a/lib/inode.c
> > > +++ b/lib/inode.c
> > > @@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT >>
> > S_SHIFT] = {
> > >
> > >  struct list_head inode_hashtable[NR_INODE_HASHTABLE];
> > >
> > > +
> > > +#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start &&
> >       \
> > > +                          roundup(end, EROFS_BLKSIZ) == end &&       \
> > > +                         (end - start) % EROFS_BLKSIZ == 0)
> > > +
> > > +/**
> > > +   read extents of the given file.
> > > +   record the data extents and link them into a chain.
> > > +   exclude holes present in file.
> > > + */
> > > +unsigned int erofs_read_sparse_extents(int fd, struct list_head
> > *extents)
> > > +{
> > > +     erofs_blk_t startblk, endblk, datablk;
> > > +     unsigned int nholes = 0;
> > > +     erofs_off_t data, hole, len = 0, last_data;
> > > +     struct erofs_sparse_extent_node *e_data;
> > > +
> > > +     len = lseek(fd, 0, SEEK_END);
> > > +     if (len < 0)
> > > +             return -errno;
> > > +     if (lseek(fd, 0, SEEK_SET) == -1)
> > > +             return -errno;
> > > +     data = 0;
> > > +     last_data = 0;
> > > +     while (data < len) {
> > > +             hole = lseek(fd, data, SEEK_HOLE);
> > > +             if (hole == len)
> > > +                     break;
> > > +             data = lseek(fd, hole, SEEK_DATA);
> > > +             if (data < 0 || hole > data)
> > > +                     return -EINVAL;
> > > +             if (IS_HOLE(hole, data)) {
> > > +                     startblk = erofs_blknr(hole);
> > > +                     datablk = erofs_blknr(last_data);
> > > +                     endblk = erofs_blknr(data);
> > > +                     last_data = data;
> > > +                     e_data = malloc(sizeof(
> > > +                                      struct erofs_sparse_extent_node));
> > > +                     if (e_data == NULL)
> > > +                             return -ENOMEM;
> > > +                     e_data->lblk = datablk;
> > > +                     e_data->len = (startblk - datablk);
> > > +                     list_add_tail(&e_data->next, extents);
> > > +                     nholes += (endblk - startblk);
> > > +             }
> > > +     }
> > > +     /* rounddown to exclude tail-end data */
> > > +     if (last_data < len && (len - last_data) >= EROFS_BLKSIZ) {
> > > +             e_data = malloc(sizeof(struct erofs_sparse_extent_node));
> > > +             if (e_data == NULL)
> > > +                     return -ENOMEM;
> > > +             startblk = erofs_blknr(last_data);
> > > +             e_data->lblk = startblk;
> > > +             e_data->len = erofs_blknr(rounddown((len - last_data),
> > > +                                       EROFS_BLKSIZ));
> > > +             list_add_tail(&e_data->next, extents);
> > > +     }
> > > +     return nholes;
> > > +}
> > > +
> > > +int erofs_write_sparse_extents(struct erofs_inode *inode, erofs_off_t
> > off)
> > > +{
> > > +     struct erofs_sparse_extent_node *e_node;
> > > +     struct erofs_sparse_extent_iheader *header;
> > > +     char *buf;
> > > +     unsigned int p = 0;
> > > +     int ret;
> > > +
> > > +     buf = malloc(inode->sparse_extent_isize);
> > > +     if (buf == NULL)
> > > +             return -ENOMEM;
> > > +     header = (struct erofs_sparse_extent_iheader *) buf;
> > > +     header->count = 0;
> > > +     p += sizeof(struct erofs_sparse_extent_iheader);
> > > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > > +             const struct erofs_sparse_extent ee = {
> > > +                     .ee_lblk = cpu_to_le32(e_node->lblk),
> > > +                     .ee_pblk = cpu_to_le32(e_node->pblk),
> > > +                     .ee_len  = cpu_to_le32(e_node->len)
> > > +             };
> > > +             memcpy(buf + p, &ee, sizeof(struct erofs_sparse_extent));
> > > +             p += sizeof(struct erofs_sparse_extent);
> > > +             header->count++;
> > > +             list_del(&e_node->next);
> > > +             free(e_node);
> > > +     }
> > > +     ret = dev_write(buf, off, inode->sparse_extent_isize);
> > > +     free(buf);
> > > +     return ret;
> > > +}
> > > +
> > >  void erofs_inode_manager_init(void)
> > >  {
> > >       unsigned int i;
> > > @@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct
> > erofs_inode *inode)
> > >
> > >  int erofs_write_file(struct erofs_inode *inode)
> > >  {
> > > -     unsigned int nblocks, i;
> > > +     unsigned int nblocks, i, j, nholes;
> > >       int ret, fd;
> > > +     struct erofs_sparse_extent_node *e_node;
> > >
> > >       if (!inode->i_size) {
> > >               inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > > @@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)
> > >       /* fallback to all data uncompressed */
> > >       inode->datalayout = EROFS_INODE_FLAT_INLINE;
> > >       nblocks = inode->i_size / EROFS_BLKSIZ;
> > > -
> > > -     ret = __allocate_inode_bh_data(inode, nblocks);
> > > -     if (ret)
> > > -             return ret;
> > > -
> > >       fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> > >       if (fd < 0)
> > >               return -errno;
> > > -
> > > -     for (i = 0; i < nblocks; ++i) {
> > > -             char buf[EROFS_BLKSIZ];
> > > -
> > > -             ret = read(fd, buf, EROFS_BLKSIZ);
> > > -             if (ret != EROFS_BLKSIZ) {
> > > -                     if (ret < 0)
> > > +     nholes = erofs_read_sparse_extents(fd, &inode->i_sparse_extents);
> > > +     if (nholes < 0) {
> > > +             close(fd);
> > > +             return nholes;
> > > +     }
> > > +     ret = __allocate_inode_bh_data(inode, nblocks - nholes);
> > > +     if (ret) {
> > > +             close(fd);
> > > +             return ret;
> > > +     }
> > > +     i = inode->u.i_blkaddr;
> > > +     inode->sparse_extent_isize = sizeof(struct
> > erofs_sparse_extent_iheader);
> > > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > > +             inode->sparse_extent_isize += sizeof(struct
> > erofs_sparse_extent);
> > > +             e_node->pblk = i;
> > > +             ret = lseek(fd, blknr_to_addr(e_node->lblk), SEEK_SET);
> > > +             if (ret < 0)
> > > +                     goto fail;
> > > +             for (j = 0; j < e_node->len; j++) {
> > > +                     char buf[EROFS_BLKSIZ];
> > > +                     ret = read(fd, buf, EROFS_BLKSIZ);
> > > +                     if (ret != EROFS_BLKSIZ) {
> > > +                             if (ret < 0)
> > > +                                     goto fail;
> > > +                             close(fd);
> > > +                             return -EAGAIN;
> > > +                     }
> > > +                     ret = blk_write(buf, e_node->pblk + j, 1);
> > > +                     if (ret)
> > >                               goto fail;
> > > -                     close(fd);
> > > -                     return -EAGAIN;
> > >               }
> > > -
> > > -             ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
> > > -             if (ret)
> > > -                     goto fail;
> > > +             i += e_node->len;
> > >       }
> > > -
> > >       /* read the tail-end data */
> > >       inode->idata_size = inode->i_size % EROFS_BLKSIZ;
> > >       if (inode->idata_size) {
> > > @@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct
> > erofs_buffer_head *bh)
> > >               if (ret)
> > >                       return false;
> > >               free(inode->compressmeta);
> > > +             off += inode->extent_isize;
> > >       }
> > >
> > > +     if (inode->sparse_extent_isize) {
> > > +             ret = erofs_write_sparse_extents(inode, off);
> > > +             if (ret)
> > > +                     return false;
> > > +     }
> > >       inode->bh = NULL;
> > >       erofs_iput(inode);
> > >       return erofs_bh_flush_generic_end(bh);
> > > @@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)
> > >
> > >       init_list_head(&inode->i_subdirs);
> > >       init_list_head(&inode->i_xattrs);
> > > +     init_list_head(&inode->i_sparse_extents);
> > >
> > >       inode->idata_size = 0;
> > >       inode->xattr_isize = 0;
> > > -     inode->extent_isize = 0;
> > > +     inode->sparse_extent_isize = 0;
> > >
> > >       inode->bh = inode->bh_inline = inode->bh_data = NULL;
> > >       inode->idata = NULL;
> > > @@ -961,4 +1071,3 @@ struct erofs_inode
> > *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
> > >
> > >       return erofs_mkfs_build_tree(inode);
> > >  }
> > > -
> > > --
> > > 2.9.3
> > >
> >

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

* Re: [RFCv3] erofs-utils: on-disk extent format for blocks
  2020-01-02 13:33     ` Gao Xiang
@ 2020-01-28  6:01       ` Pratik Shinde
  2020-01-28  6:37         ` Gao Xiang via Linux-erofs
  0 siblings, 1 reply; 6+ messages in thread
From: Pratik Shinde @ 2020-01-28  6:01 UTC (permalink / raw)
  To: Gao Xiang; +Cc: miaoxie, linux-erofs

[-- Attachment #1: Type: text/plain, Size: 13446 bytes --]

Hello Gao,

I have started working on the on kernel side RFC patch of this change , is
this still on erofs roadmap ?  Will you able to review it?

--Pratik.

On Thu, Jan 2, 2020, 7:03 PM Gao Xiang <gaoxiang25@huawei.com> wrote:

> On Thu, Jan 02, 2020 at 04:32:34PM +0530, Pratik Shinde wrote:
> > Hi Gao,
> >
> > You are correct. The macro logic can be simplified. I will do that.
> > I will work on the kernel part of this change & do some testing on it.
> > I will keep you posted about the change and relevant tests I am running
> on
> > it.
> >
> > --Pratik.
> >
>
> Okay, thanks for your effort. :)
>
> Thanks,
> Gao Xiang
>
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > On Thu, Jan 2, 2020, 4:17 PM Gao Xiang <gaoxiang25@huawei.com> wrote:
> >
> > > Hi Pratik,
> > >
> > > On Thu, Jan 02, 2020 at 03:17:32PM +0530, Pratik Shinde wrote:
> > > > 1)Moved on-disk structures to erofs_fs.h
> > > > 2)Some naming changes.
> > > >
> > > > I think we can keep 'IS_HOLE()' macro, otherwise the code
> > > > does not look clean(if used directly/without macro).Its getting
> > > > used only in inode.c so it can be kept there.
> > > > what do you think ?
> > >
> > > What I'm little concerned is the relationship between
> > > the name of IS_HOLE and its implementation...
> > >
> > > In other words, why
> > >  roundup(start, EROFS_BLKSIZ) == start &&
> > >  roundup(end, EROFS_BLKSIZ) == end &&
> > >  (end - start) % EROFS_BLKSIZ == 0
> > > should be an erofs hole...
> > >
> > > But that is minor, I reserve my opinion on this for now...
> > >
> > > This patch generally looks good to me, yet I haven't
> > > played with it till now.
> > >
> > > Could you implement a workable RFC patch for kernel side
> > > as well? Since I'm still busying in XZ library and other
> > > convert patches for 5.6...
> > >
> > > I'd like to hear if some other opinions from Chao and Guifu.
> > > Since it enhances the on-disk format, we need to think it
> > > over (especially its future expandability).
> > >
> > >
> > > To Chao and Guifu,
> > > Could you have some extra time looking at this stuff as well?
> > >
> > >
> > > Thanks,
> > > Gao Xiang
> > >
> > > >
> > > > Signed-off-by: Pratik Shinde <pratikshinde320@gmail.com>
> > > > ---
> > > >  include/erofs/internal.h |   9 ++-
> > > >  include/erofs_fs.h       |  11 ++++
> > > >  lib/inode.c              | 153
> > > ++++++++++++++++++++++++++++++++++++++++-------
> > > >  3 files changed, 150 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > > index e13adda..2d7466b 100644
> > > > --- a/include/erofs/internal.h
> > > > +++ b/include/erofs/internal.h
> > > > @@ -63,7 +63,7 @@ struct erofs_sb_info {
> > > >  extern struct erofs_sb_info sbi;
> > > >
> > > >  struct erofs_inode {
> > > > -     struct list_head i_hash, i_subdirs, i_xattrs;
> > > > +     struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;
> > > >
> > > >       unsigned int i_count;
> > > >       struct erofs_inode *i_parent;
> > > > @@ -93,6 +93,7 @@ struct erofs_inode {
> > > >
> > > >       unsigned int xattr_isize;
> > > >       unsigned int extent_isize;
> > > > +     unsigned int sparse_extent_isize;
> > > >
> > > >       erofs_nid_t nid;
> > > >       struct erofs_buffer_head *bh;
> > > > @@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int
> err)
> > > >       return msg;
> > > >  }
> > > >
> > > > +struct erofs_sparse_extent_node {
> > > > +     struct list_head next;
> > > > +     erofs_blk_t lblk;
> > > > +     erofs_blk_t pblk;
> > > > +     u32 len;
> > > > +};
> > > >  #endif
> > > >
> > > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > > index bcc4f0c..a63e1c6 100644
> > > > --- a/include/erofs_fs.h
> > > > +++ b/include/erofs_fs.h
> > > > @@ -321,5 +321,16 @@ static inline void
> > > erofs_check_ondisk_layout_definitions(void)
> > > >                    Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > > >  }
> > > >
> > > > +/* on-disk sparse extent format */
> > > > +struct erofs_sparse_extent {
> > > > +     __le32 ee_lblk;
> > > > +     __le32 ee_pblk;
> > > > +     __le32 ee_len;
> > > > +};
> > > > +
> > > > +struct erofs_sparse_extent_iheader {
> > > > +     u32 count;
> > > > +};
> > > > +
> > > >  #endif
> > > >
> > > > diff --git a/lib/inode.c b/lib/inode.c
> > > > index 0e19b11..da20599 100644
> > > > --- a/lib/inode.c
> > > > +++ b/lib/inode.c
> > > > @@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT >>
> > > S_SHIFT] = {
> > > >
> > > >  struct list_head inode_hashtable[NR_INODE_HASHTABLE];
> > > >
> > > > +
> > > > +#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start
> &&
> > >       \
> > > > +                          roundup(end, EROFS_BLKSIZ) == end &&
>  \
> > > > +                         (end - start) % EROFS_BLKSIZ == 0)
> > > > +
> > > > +/**
> > > > +   read extents of the given file.
> > > > +   record the data extents and link them into a chain.
> > > > +   exclude holes present in file.
> > > > + */
> > > > +unsigned int erofs_read_sparse_extents(int fd, struct list_head
> > > *extents)
> > > > +{
> > > > +     erofs_blk_t startblk, endblk, datablk;
> > > > +     unsigned int nholes = 0;
> > > > +     erofs_off_t data, hole, len = 0, last_data;
> > > > +     struct erofs_sparse_extent_node *e_data;
> > > > +
> > > > +     len = lseek(fd, 0, SEEK_END);
> > > > +     if (len < 0)
> > > > +             return -errno;
> > > > +     if (lseek(fd, 0, SEEK_SET) == -1)
> > > > +             return -errno;
> > > > +     data = 0;
> > > > +     last_data = 0;
> > > > +     while (data < len) {
> > > > +             hole = lseek(fd, data, SEEK_HOLE);
> > > > +             if (hole == len)
> > > > +                     break;
> > > > +             data = lseek(fd, hole, SEEK_DATA);
> > > > +             if (data < 0 || hole > data)
> > > > +                     return -EINVAL;
> > > > +             if (IS_HOLE(hole, data)) {
> > > > +                     startblk = erofs_blknr(hole);
> > > > +                     datablk = erofs_blknr(last_data);
> > > > +                     endblk = erofs_blknr(data);
> > > > +                     last_data = data;
> > > > +                     e_data = malloc(sizeof(
> > > > +                                      struct
> erofs_sparse_extent_node));
> > > > +                     if (e_data == NULL)
> > > > +                             return -ENOMEM;
> > > > +                     e_data->lblk = datablk;
> > > > +                     e_data->len = (startblk - datablk);
> > > > +                     list_add_tail(&e_data->next, extents);
> > > > +                     nholes += (endblk - startblk);
> > > > +             }
> > > > +     }
> > > > +     /* rounddown to exclude tail-end data */
> > > > +     if (last_data < len && (len - last_data) >= EROFS_BLKSIZ) {
> > > > +             e_data = malloc(sizeof(struct
> erofs_sparse_extent_node));
> > > > +             if (e_data == NULL)
> > > > +                     return -ENOMEM;
> > > > +             startblk = erofs_blknr(last_data);
> > > > +             e_data->lblk = startblk;
> > > > +             e_data->len = erofs_blknr(rounddown((len - last_data),
> > > > +                                       EROFS_BLKSIZ));
> > > > +             list_add_tail(&e_data->next, extents);
> > > > +     }
> > > > +     return nholes;
> > > > +}
> > > > +
> > > > +int erofs_write_sparse_extents(struct erofs_inode *inode,
> erofs_off_t
> > > off)
> > > > +{
> > > > +     struct erofs_sparse_extent_node *e_node;
> > > > +     struct erofs_sparse_extent_iheader *header;
> > > > +     char *buf;
> > > > +     unsigned int p = 0;
> > > > +     int ret;
> > > > +
> > > > +     buf = malloc(inode->sparse_extent_isize);
> > > > +     if (buf == NULL)
> > > > +             return -ENOMEM;
> > > > +     header = (struct erofs_sparse_extent_iheader *) buf;
> > > > +     header->count = 0;
> > > > +     p += sizeof(struct erofs_sparse_extent_iheader);
> > > > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > > > +             const struct erofs_sparse_extent ee = {
> > > > +                     .ee_lblk = cpu_to_le32(e_node->lblk),
> > > > +                     .ee_pblk = cpu_to_le32(e_node->pblk),
> > > > +                     .ee_len  = cpu_to_le32(e_node->len)
> > > > +             };
> > > > +             memcpy(buf + p, &ee, sizeof(struct
> erofs_sparse_extent));
> > > > +             p += sizeof(struct erofs_sparse_extent);
> > > > +             header->count++;
> > > > +             list_del(&e_node->next);
> > > > +             free(e_node);
> > > > +     }
> > > > +     ret = dev_write(buf, off, inode->sparse_extent_isize);
> > > > +     free(buf);
> > > > +     return ret;
> > > > +}
> > > > +
> > > >  void erofs_inode_manager_init(void)
> > > >  {
> > > >       unsigned int i;
> > > > @@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct
> > > erofs_inode *inode)
> > > >
> > > >  int erofs_write_file(struct erofs_inode *inode)
> > > >  {
> > > > -     unsigned int nblocks, i;
> > > > +     unsigned int nblocks, i, j, nholes;
> > > >       int ret, fd;
> > > > +     struct erofs_sparse_extent_node *e_node;
> > > >
> > > >       if (!inode->i_size) {
> > > >               inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > > > @@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)
> > > >       /* fallback to all data uncompressed */
> > > >       inode->datalayout = EROFS_INODE_FLAT_INLINE;
> > > >       nblocks = inode->i_size / EROFS_BLKSIZ;
> > > > -
> > > > -     ret = __allocate_inode_bh_data(inode, nblocks);
> > > > -     if (ret)
> > > > -             return ret;
> > > > -
> > > >       fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> > > >       if (fd < 0)
> > > >               return -errno;
> > > > -
> > > > -     for (i = 0; i < nblocks; ++i) {
> > > > -             char buf[EROFS_BLKSIZ];
> > > > -
> > > > -             ret = read(fd, buf, EROFS_BLKSIZ);
> > > > -             if (ret != EROFS_BLKSIZ) {
> > > > -                     if (ret < 0)
> > > > +     nholes = erofs_read_sparse_extents(fd,
> &inode->i_sparse_extents);
> > > > +     if (nholes < 0) {
> > > > +             close(fd);
> > > > +             return nholes;
> > > > +     }
> > > > +     ret = __allocate_inode_bh_data(inode, nblocks - nholes);
> > > > +     if (ret) {
> > > > +             close(fd);
> > > > +             return ret;
> > > > +     }
> > > > +     i = inode->u.i_blkaddr;
> > > > +     inode->sparse_extent_isize = sizeof(struct
> > > erofs_sparse_extent_iheader);
> > > > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > > > +             inode->sparse_extent_isize += sizeof(struct
> > > erofs_sparse_extent);
> > > > +             e_node->pblk = i;
> > > > +             ret = lseek(fd, blknr_to_addr(e_node->lblk), SEEK_SET);
> > > > +             if (ret < 0)
> > > > +                     goto fail;
> > > > +             for (j = 0; j < e_node->len; j++) {
> > > > +                     char buf[EROFS_BLKSIZ];
> > > > +                     ret = read(fd, buf, EROFS_BLKSIZ);
> > > > +                     if (ret != EROFS_BLKSIZ) {
> > > > +                             if (ret < 0)
> > > > +                                     goto fail;
> > > > +                             close(fd);
> > > > +                             return -EAGAIN;
> > > > +                     }
> > > > +                     ret = blk_write(buf, e_node->pblk + j, 1);
> > > > +                     if (ret)
> > > >                               goto fail;
> > > > -                     close(fd);
> > > > -                     return -EAGAIN;
> > > >               }
> > > > -
> > > > -             ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
> > > > -             if (ret)
> > > > -                     goto fail;
> > > > +             i += e_node->len;
> > > >       }
> > > > -
> > > >       /* read the tail-end data */
> > > >       inode->idata_size = inode->i_size % EROFS_BLKSIZ;
> > > >       if (inode->idata_size) {
> > > > @@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct
> > > erofs_buffer_head *bh)
> > > >               if (ret)
> > > >                       return false;
> > > >               free(inode->compressmeta);
> > > > +             off += inode->extent_isize;
> > > >       }
> > > >
> > > > +     if (inode->sparse_extent_isize) {
> > > > +             ret = erofs_write_sparse_extents(inode, off);
> > > > +             if (ret)
> > > > +                     return false;
> > > > +     }
> > > >       inode->bh = NULL;
> > > >       erofs_iput(inode);
> > > >       return erofs_bh_flush_generic_end(bh);
> > > > @@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)
> > > >
> > > >       init_list_head(&inode->i_subdirs);
> > > >       init_list_head(&inode->i_xattrs);
> > > > +     init_list_head(&inode->i_sparse_extents);
> > > >
> > > >       inode->idata_size = 0;
> > > >       inode->xattr_isize = 0;
> > > > -     inode->extent_isize = 0;
> > > > +     inode->sparse_extent_isize = 0;
> > > >
> > > >       inode->bh = inode->bh_inline = inode->bh_data = NULL;
> > > >       inode->idata = NULL;
> > > > @@ -961,4 +1071,3 @@ struct erofs_inode
> > > *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
> > > >
> > > >       return erofs_mkfs_build_tree(inode);
> > > >  }
> > > > -
> > > > --
> > > > 2.9.3
> > > >
> > >
>

[-- Attachment #2: Type: text/html, Size: 19360 bytes --]

<div dir="auto">Hello Gao,<div dir="auto"><br></div><div dir="auto">I have started working on the on kernel side RFC patch of this change , is this still on erofs roadmap ?  Will you able to review it?</div><div dir="auto"><br></div><div dir="auto">--Pratik.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 2, 2020, 7:03 PM Gao Xiang &lt;<a href="mailto:gaoxiang25@huawei.com">gaoxiang25@huawei.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, Jan 02, 2020 at 04:32:34PM +0530, Pratik Shinde wrote:<br>
&gt; Hi Gao,<br>
&gt; <br>
&gt; You are correct. The macro logic can be simplified. I will do that.<br>
&gt; I will work on the kernel part of this change &amp; do some testing on it.<br>
&gt; I will keep you posted about the change and relevant tests I am running on<br>
&gt; it.<br>
&gt; <br>
&gt; --Pratik.<br>
&gt;<br>
<br>
Okay, thanks for your effort. :)<br>
<br>
Thanks,<br>
Gao Xiang<br>
<br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; On Thu, Jan 2, 2020, 4:17 PM Gao Xiang &lt;<a href="mailto:gaoxiang25@huawei.com" target="_blank" rel="noreferrer">gaoxiang25@huawei.com</a>&gt; wrote:<br>
&gt; <br>
&gt; &gt; Hi Pratik,<br>
&gt; &gt;<br>
&gt; &gt; On Thu, Jan 02, 2020 at 03:17:32PM +0530, Pratik Shinde wrote:<br>
&gt; &gt; &gt; 1)Moved on-disk structures to erofs_fs.h<br>
&gt; &gt; &gt; 2)Some naming changes.<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; I think we can keep &#39;IS_HOLE()&#39; macro, otherwise the code<br>
&gt; &gt; &gt; does not look clean(if used directly/without macro).Its getting<br>
&gt; &gt; &gt; used only in inode.c so it can be kept there.<br>
&gt; &gt; &gt; what do you think ?<br>
&gt; &gt;<br>
&gt; &gt; What I&#39;m little concerned is the relationship between<br>
&gt; &gt; the name of IS_HOLE and its implementation...<br>
&gt; &gt;<br>
&gt; &gt; In other words, why<br>
&gt; &gt;  roundup(start, EROFS_BLKSIZ) == start &amp;&amp;<br>
&gt; &gt;  roundup(end, EROFS_BLKSIZ) == end &amp;&amp;<br>
&gt; &gt;  (end - start) % EROFS_BLKSIZ == 0<br>
&gt; &gt; should be an erofs hole...<br>
&gt; &gt;<br>
&gt; &gt; But that is minor, I reserve my opinion on this for now...<br>
&gt; &gt;<br>
&gt; &gt; This patch generally looks good to me, yet I haven&#39;t<br>
&gt; &gt; played with it till now.<br>
&gt; &gt;<br>
&gt; &gt; Could you implement a workable RFC patch for kernel side<br>
&gt; &gt; as well? Since I&#39;m still busying in XZ library and other<br>
&gt; &gt; convert patches for 5.6...<br>
&gt; &gt;<br>
&gt; &gt; I&#39;d like to hear if some other opinions from Chao and Guifu.<br>
&gt; &gt; Since it enhances the on-disk format, we need to think it<br>
&gt; &gt; over (especially its future expandability).<br>
&gt; &gt;<br>
&gt; &gt;<br>
&gt; &gt; To Chao and Guifu,<br>
&gt; &gt; Could you have some extra time looking at this stuff as well?<br>
&gt; &gt;<br>
&gt; &gt;<br>
&gt; &gt; Thanks,<br>
&gt; &gt; Gao Xiang<br>
&gt; &gt;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; Signed-off-by: Pratik Shinde &lt;<a href="mailto:pratikshinde320@gmail.com" target="_blank" rel="noreferrer">pratikshinde320@gmail.com</a>&gt;<br>
&gt; &gt; &gt; ---<br>
&gt; &gt; &gt;  include/erofs/internal.h |   9 ++-<br>
&gt; &gt; &gt;  include/erofs_fs.h       |  11 ++++<br>
&gt; &gt; &gt;  lib/inode.c              | 153<br>
&gt; &gt; ++++++++++++++++++++++++++++++++++++++++-------<br>
&gt; &gt; &gt;  3 files changed, 150 insertions(+), 23 deletions(-)<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; diff --git a/include/erofs/internal.h b/include/erofs/internal.h<br>
&gt; &gt; &gt; index e13adda..2d7466b 100644<br>
&gt; &gt; &gt; --- a/include/erofs/internal.h<br>
&gt; &gt; &gt; +++ b/include/erofs/internal.h<br>
&gt; &gt; &gt; @@ -63,7 +63,7 @@ struct erofs_sb_info {<br>
&gt; &gt; &gt;  extern struct erofs_sb_info sbi;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;  struct erofs_inode {<br>
&gt; &gt; &gt; -     struct list_head i_hash, i_subdirs, i_xattrs;<br>
&gt; &gt; &gt; +     struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       unsigned int i_count;<br>
&gt; &gt; &gt;       struct erofs_inode *i_parent;<br>
&gt; &gt; &gt; @@ -93,6 +93,7 @@ struct erofs_inode {<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       unsigned int xattr_isize;<br>
&gt; &gt; &gt;       unsigned int extent_isize;<br>
&gt; &gt; &gt; +     unsigned int sparse_extent_isize;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       erofs_nid_t nid;<br>
&gt; &gt; &gt;       struct erofs_buffer_head *bh;<br>
&gt; &gt; &gt; @@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int err)<br>
&gt; &gt; &gt;       return msg;<br>
&gt; &gt; &gt;  }<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; +struct erofs_sparse_extent_node {<br>
&gt; &gt; &gt; +     struct list_head next;<br>
&gt; &gt; &gt; +     erofs_blk_t lblk;<br>
&gt; &gt; &gt; +     erofs_blk_t pblk;<br>
&gt; &gt; &gt; +     u32 len;<br>
&gt; &gt; &gt; +};<br>
&gt; &gt; &gt;  #endif<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; diff --git a/include/erofs_fs.h b/include/erofs_fs.h<br>
&gt; &gt; &gt; index bcc4f0c..a63e1c6 100644<br>
&gt; &gt; &gt; --- a/include/erofs_fs.h<br>
&gt; &gt; &gt; +++ b/include/erofs_fs.h<br>
&gt; &gt; &gt; @@ -321,5 +321,16 @@ static inline void<br>
&gt; &gt; erofs_check_ondisk_layout_definitions(void)<br>
&gt; &gt; &gt;                    Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);<br>
&gt; &gt; &gt;  }<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; +/* on-disk sparse extent format */<br>
&gt; &gt; &gt; +struct erofs_sparse_extent {<br>
&gt; &gt; &gt; +     __le32 ee_lblk;<br>
&gt; &gt; &gt; +     __le32 ee_pblk;<br>
&gt; &gt; &gt; +     __le32 ee_len;<br>
&gt; &gt; &gt; +};<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt; +struct erofs_sparse_extent_iheader {<br>
&gt; &gt; &gt; +     u32 count;<br>
&gt; &gt; &gt; +};<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt;  #endif<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; diff --git a/lib/inode.c b/lib/inode.c<br>
&gt; &gt; &gt; index 0e19b11..da20599 100644<br>
&gt; &gt; &gt; --- a/lib/inode.c<br>
&gt; &gt; &gt; +++ b/lib/inode.c<br>
&gt; &gt; &gt; @@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT &gt;&gt;<br>
&gt; &gt; S_SHIFT] = {<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;  struct list_head inode_hashtable[NR_INODE_HASHTABLE];<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt; +#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start &amp;&amp;<br>
&gt; &gt;       \<br>
&gt; &gt; &gt; +                          roundup(end, EROFS_BLKSIZ) == end &amp;&amp;       \<br>
&gt; &gt; &gt; +                         (end - start) % EROFS_BLKSIZ == 0)<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt; +/**<br>
&gt; &gt; &gt; +   read extents of the given file.<br>
&gt; &gt; &gt; +   record the data extents and link them into a chain.<br>
&gt; &gt; &gt; +   exclude holes present in file.<br>
&gt; &gt; &gt; + */<br>
&gt; &gt; &gt; +unsigned int erofs_read_sparse_extents(int fd, struct list_head<br>
&gt; &gt; *extents)<br>
&gt; &gt; &gt; +{<br>
&gt; &gt; &gt; +     erofs_blk_t startblk, endblk, datablk;<br>
&gt; &gt; &gt; +     unsigned int nholes = 0;<br>
&gt; &gt; &gt; +     erofs_off_t data, hole, len = 0, last_data;<br>
&gt; &gt; &gt; +     struct erofs_sparse_extent_node *e_data;<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt; +     len = lseek(fd, 0, SEEK_END);<br>
&gt; &gt; &gt; +     if (len &lt; 0)<br>
&gt; &gt; &gt; +             return -errno;<br>
&gt; &gt; &gt; +     if (lseek(fd, 0, SEEK_SET) == -1)<br>
&gt; &gt; &gt; +             return -errno;<br>
&gt; &gt; &gt; +     data = 0;<br>
&gt; &gt; &gt; +     last_data = 0;<br>
&gt; &gt; &gt; +     while (data &lt; len) {<br>
&gt; &gt; &gt; +             hole = lseek(fd, data, SEEK_HOLE);<br>
&gt; &gt; &gt; +             if (hole == len)<br>
&gt; &gt; &gt; +                     break;<br>
&gt; &gt; &gt; +             data = lseek(fd, hole, SEEK_DATA);<br>
&gt; &gt; &gt; +             if (data &lt; 0 || hole &gt; data)<br>
&gt; &gt; &gt; +                     return -EINVAL;<br>
&gt; &gt; &gt; +             if (IS_HOLE(hole, data)) {<br>
&gt; &gt; &gt; +                     startblk = erofs_blknr(hole);<br>
&gt; &gt; &gt; +                     datablk = erofs_blknr(last_data);<br>
&gt; &gt; &gt; +                     endblk = erofs_blknr(data);<br>
&gt; &gt; &gt; +                     last_data = data;<br>
&gt; &gt; &gt; +                     e_data = malloc(sizeof(<br>
&gt; &gt; &gt; +                                      struct erofs_sparse_extent_node));<br>
&gt; &gt; &gt; +                     if (e_data == NULL)<br>
&gt; &gt; &gt; +                             return -ENOMEM;<br>
&gt; &gt; &gt; +                     e_data-&gt;lblk = datablk;<br>
&gt; &gt; &gt; +                     e_data-&gt;len = (startblk - datablk);<br>
&gt; &gt; &gt; +                     list_add_tail(&amp;e_data-&gt;next, extents);<br>
&gt; &gt; &gt; +                     nholes += (endblk - startblk);<br>
&gt; &gt; &gt; +             }<br>
&gt; &gt; &gt; +     }<br>
&gt; &gt; &gt; +     /* rounddown to exclude tail-end data */<br>
&gt; &gt; &gt; +     if (last_data &lt; len &amp;&amp; (len - last_data) &gt;= EROFS_BLKSIZ) {<br>
&gt; &gt; &gt; +             e_data = malloc(sizeof(struct erofs_sparse_extent_node));<br>
&gt; &gt; &gt; +             if (e_data == NULL)<br>
&gt; &gt; &gt; +                     return -ENOMEM;<br>
&gt; &gt; &gt; +             startblk = erofs_blknr(last_data);<br>
&gt; &gt; &gt; +             e_data-&gt;lblk = startblk;<br>
&gt; &gt; &gt; +             e_data-&gt;len = erofs_blknr(rounddown((len - last_data),<br>
&gt; &gt; &gt; +                                       EROFS_BLKSIZ));<br>
&gt; &gt; &gt; +             list_add_tail(&amp;e_data-&gt;next, extents);<br>
&gt; &gt; &gt; +     }<br>
&gt; &gt; &gt; +     return nholes;<br>
&gt; &gt; &gt; +}<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt; +int erofs_write_sparse_extents(struct erofs_inode *inode, erofs_off_t<br>
&gt; &gt; off)<br>
&gt; &gt; &gt; +{<br>
&gt; &gt; &gt; +     struct erofs_sparse_extent_node *e_node;<br>
&gt; &gt; &gt; +     struct erofs_sparse_extent_iheader *header;<br>
&gt; &gt; &gt; +     char *buf;<br>
&gt; &gt; &gt; +     unsigned int p = 0;<br>
&gt; &gt; &gt; +     int ret;<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt; +     buf = malloc(inode-&gt;sparse_extent_isize);<br>
&gt; &gt; &gt; +     if (buf == NULL)<br>
&gt; &gt; &gt; +             return -ENOMEM;<br>
&gt; &gt; &gt; +     header = (struct erofs_sparse_extent_iheader *) buf;<br>
&gt; &gt; &gt; +     header-&gt;count = 0;<br>
&gt; &gt; &gt; +     p += sizeof(struct erofs_sparse_extent_iheader);<br>
&gt; &gt; &gt; +     list_for_each_entry(e_node, &amp;inode-&gt;i_sparse_extents, next) {<br>
&gt; &gt; &gt; +             const struct erofs_sparse_extent ee = {<br>
&gt; &gt; &gt; +                     .ee_lblk = cpu_to_le32(e_node-&gt;lblk),<br>
&gt; &gt; &gt; +                     .ee_pblk = cpu_to_le32(e_node-&gt;pblk),<br>
&gt; &gt; &gt; +                     .ee_len  = cpu_to_le32(e_node-&gt;len)<br>
&gt; &gt; &gt; +             };<br>
&gt; &gt; &gt; +             memcpy(buf + p, &amp;ee, sizeof(struct erofs_sparse_extent));<br>
&gt; &gt; &gt; +             p += sizeof(struct erofs_sparse_extent);<br>
&gt; &gt; &gt; +             header-&gt;count++;<br>
&gt; &gt; &gt; +             list_del(&amp;e_node-&gt;next);<br>
&gt; &gt; &gt; +             free(e_node);<br>
&gt; &gt; &gt; +     }<br>
&gt; &gt; &gt; +     ret = dev_write(buf, off, inode-&gt;sparse_extent_isize);<br>
&gt; &gt; &gt; +     free(buf);<br>
&gt; &gt; &gt; +     return ret;<br>
&gt; &gt; &gt; +}<br>
&gt; &gt; &gt; +<br>
&gt; &gt; &gt;  void erofs_inode_manager_init(void)<br>
&gt; &gt; &gt;  {<br>
&gt; &gt; &gt;       unsigned int i;<br>
&gt; &gt; &gt; @@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct<br>
&gt; &gt; erofs_inode *inode)<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;  int erofs_write_file(struct erofs_inode *inode)<br>
&gt; &gt; &gt;  {<br>
&gt; &gt; &gt; -     unsigned int nblocks, i;<br>
&gt; &gt; &gt; +     unsigned int nblocks, i, j, nholes;<br>
&gt; &gt; &gt;       int ret, fd;<br>
&gt; &gt; &gt; +     struct erofs_sparse_extent_node *e_node;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       if (!inode-&gt;i_size) {<br>
&gt; &gt; &gt;               inode-&gt;datalayout = EROFS_INODE_FLAT_PLAIN;<br>
&gt; &gt; &gt; @@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)<br>
&gt; &gt; &gt;       /* fallback to all data uncompressed */<br>
&gt; &gt; &gt;       inode-&gt;datalayout = EROFS_INODE_FLAT_INLINE;<br>
&gt; &gt; &gt;       nblocks = inode-&gt;i_size / EROFS_BLKSIZ;<br>
&gt; &gt; &gt; -<br>
&gt; &gt; &gt; -     ret = __allocate_inode_bh_data(inode, nblocks);<br>
&gt; &gt; &gt; -     if (ret)<br>
&gt; &gt; &gt; -             return ret;<br>
&gt; &gt; &gt; -<br>
&gt; &gt; &gt;       fd = open(inode-&gt;i_srcpath, O_RDONLY | O_BINARY);<br>
&gt; &gt; &gt;       if (fd &lt; 0)<br>
&gt; &gt; &gt;               return -errno;<br>
&gt; &gt; &gt; -<br>
&gt; &gt; &gt; -     for (i = 0; i &lt; nblocks; ++i) {<br>
&gt; &gt; &gt; -             char buf[EROFS_BLKSIZ];<br>
&gt; &gt; &gt; -<br>
&gt; &gt; &gt; -             ret = read(fd, buf, EROFS_BLKSIZ);<br>
&gt; &gt; &gt; -             if (ret != EROFS_BLKSIZ) {<br>
&gt; &gt; &gt; -                     if (ret &lt; 0)<br>
&gt; &gt; &gt; +     nholes = erofs_read_sparse_extents(fd, &amp;inode-&gt;i_sparse_extents);<br>
&gt; &gt; &gt; +     if (nholes &lt; 0) {<br>
&gt; &gt; &gt; +             close(fd);<br>
&gt; &gt; &gt; +             return nholes;<br>
&gt; &gt; &gt; +     }<br>
&gt; &gt; &gt; +     ret = __allocate_inode_bh_data(inode, nblocks - nholes);<br>
&gt; &gt; &gt; +     if (ret) {<br>
&gt; &gt; &gt; +             close(fd);<br>
&gt; &gt; &gt; +             return ret;<br>
&gt; &gt; &gt; +     }<br>
&gt; &gt; &gt; +     i = inode-&gt;u.i_blkaddr;<br>
&gt; &gt; &gt; +     inode-&gt;sparse_extent_isize = sizeof(struct<br>
&gt; &gt; erofs_sparse_extent_iheader);<br>
&gt; &gt; &gt; +     list_for_each_entry(e_node, &amp;inode-&gt;i_sparse_extents, next) {<br>
&gt; &gt; &gt; +             inode-&gt;sparse_extent_isize += sizeof(struct<br>
&gt; &gt; erofs_sparse_extent);<br>
&gt; &gt; &gt; +             e_node-&gt;pblk = i;<br>
&gt; &gt; &gt; +             ret = lseek(fd, blknr_to_addr(e_node-&gt;lblk), SEEK_SET);<br>
&gt; &gt; &gt; +             if (ret &lt; 0)<br>
&gt; &gt; &gt; +                     goto fail;<br>
&gt; &gt; &gt; +             for (j = 0; j &lt; e_node-&gt;len; j++) {<br>
&gt; &gt; &gt; +                     char buf[EROFS_BLKSIZ];<br>
&gt; &gt; &gt; +                     ret = read(fd, buf, EROFS_BLKSIZ);<br>
&gt; &gt; &gt; +                     if (ret != EROFS_BLKSIZ) {<br>
&gt; &gt; &gt; +                             if (ret &lt; 0)<br>
&gt; &gt; &gt; +                                     goto fail;<br>
&gt; &gt; &gt; +                             close(fd);<br>
&gt; &gt; &gt; +                             return -EAGAIN;<br>
&gt; &gt; &gt; +                     }<br>
&gt; &gt; &gt; +                     ret = blk_write(buf, e_node-&gt;pblk + j, 1);<br>
&gt; &gt; &gt; +                     if (ret)<br>
&gt; &gt; &gt;                               goto fail;<br>
&gt; &gt; &gt; -                     close(fd);<br>
&gt; &gt; &gt; -                     return -EAGAIN;<br>
&gt; &gt; &gt;               }<br>
&gt; &gt; &gt; -<br>
&gt; &gt; &gt; -             ret = blk_write(buf, inode-&gt;u.i_blkaddr + i, 1);<br>
&gt; &gt; &gt; -             if (ret)<br>
&gt; &gt; &gt; -                     goto fail;<br>
&gt; &gt; &gt; +             i += e_node-&gt;len;<br>
&gt; &gt; &gt;       }<br>
&gt; &gt; &gt; -<br>
&gt; &gt; &gt;       /* read the tail-end data */<br>
&gt; &gt; &gt;       inode-&gt;idata_size = inode-&gt;i_size % EROFS_BLKSIZ;<br>
&gt; &gt; &gt;       if (inode-&gt;idata_size) {<br>
&gt; &gt; &gt; @@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct<br>
&gt; &gt; erofs_buffer_head *bh)<br>
&gt; &gt; &gt;               if (ret)<br>
&gt; &gt; &gt;                       return false;<br>
&gt; &gt; &gt;               free(inode-&gt;compressmeta);<br>
&gt; &gt; &gt; +             off += inode-&gt;extent_isize;<br>
&gt; &gt; &gt;       }<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; +     if (inode-&gt;sparse_extent_isize) {<br>
&gt; &gt; &gt; +             ret = erofs_write_sparse_extents(inode, off);<br>
&gt; &gt; &gt; +             if (ret)<br>
&gt; &gt; &gt; +                     return false;<br>
&gt; &gt; &gt; +     }<br>
&gt; &gt; &gt;       inode-&gt;bh = NULL;<br>
&gt; &gt; &gt;       erofs_iput(inode);<br>
&gt; &gt; &gt;       return erofs_bh_flush_generic_end(bh);<br>
&gt; &gt; &gt; @@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       init_list_head(&amp;inode-&gt;i_subdirs);<br>
&gt; &gt; &gt;       init_list_head(&amp;inode-&gt;i_xattrs);<br>
&gt; &gt; &gt; +     init_list_head(&amp;inode-&gt;i_sparse_extents);<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       inode-&gt;idata_size = 0;<br>
&gt; &gt; &gt;       inode-&gt;xattr_isize = 0;<br>
&gt; &gt; &gt; -     inode-&gt;extent_isize = 0;<br>
&gt; &gt; &gt; +     inode-&gt;sparse_extent_isize = 0;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       inode-&gt;bh = inode-&gt;bh_inline = inode-&gt;bh_data = NULL;<br>
&gt; &gt; &gt;       inode-&gt;idata = NULL;<br>
&gt; &gt; &gt; @@ -961,4 +1071,3 @@ struct erofs_inode<br>
&gt; &gt; *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;       return erofs_mkfs_build_tree(inode);<br>
&gt; &gt; &gt;  }<br>
&gt; &gt; &gt; -<br>
&gt; &gt; &gt; --<br>
&gt; &gt; &gt; 2.9.3<br>
&gt; &gt; &gt;<br>
&gt; &gt;<br>
</blockquote></div>

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

* Re: [RFCv3] erofs-utils: on-disk extent format for blocks
  2020-01-28  6:01       ` Pratik Shinde
@ 2020-01-28  6:37         ` Gao Xiang via Linux-erofs
  0 siblings, 0 replies; 6+ messages in thread
From: Gao Xiang via Linux-erofs @ 2020-01-28  6:37 UTC (permalink / raw)
  To: Pratik Shinde; +Cc: miaoxie, linux-erofs

Hi Pratik,

On Tue, Jan 28, 2020 at 11:31:29AM +0530, Pratik Shinde wrote:
> Hello Gao,
> 
> I have started working on the on kernel side RFC patch of this change , is
> this still on erofs roadmap ?  Will you able to review it?

Yes, of course. You could send out when it gets ready. All chinese people
are spending Chinese new year festival in hometown, as well as taking action
to keep away from Wuhan pneumonia now.

Thanks,
Gao Xiang

> 
> --Pratik.
> 
> On Thu, Jan 2, 2020, 7:03 PM Gao Xiang <gaoxiang25@huawei.com> wrote:
> 
> > On Thu, Jan 02, 2020 at 04:32:34PM +0530, Pratik Shinde wrote:
> > > Hi Gao,
> > >
> > > You are correct. The macro logic can be simplified. I will do that.
> > > I will work on the kernel part of this change & do some testing on it.
> > > I will keep you posted about the change and relevant tests I am running
> > on
> > > it.
> > >
> > > --Pratik.
> > >
> >
> > Okay, thanks for your effort. :)
> >
> > Thanks,
> > Gao Xiang
> >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > On Thu, Jan 2, 2020, 4:17 PM Gao Xiang <gaoxiang25@huawei.com> wrote:
> > >
> > > > Hi Pratik,
> > > >
> > > > On Thu, Jan 02, 2020 at 03:17:32PM +0530, Pratik Shinde wrote:
> > > > > 1)Moved on-disk structures to erofs_fs.h
> > > > > 2)Some naming changes.
> > > > >
> > > > > I think we can keep 'IS_HOLE()' macro, otherwise the code
> > > > > does not look clean(if used directly/without macro).Its getting
> > > > > used only in inode.c so it can be kept there.
> > > > > what do you think ?
> > > >
> > > > What I'm little concerned is the relationship between
> > > > the name of IS_HOLE and its implementation...
> > > >
> > > > In other words, why
> > > >  roundup(start, EROFS_BLKSIZ) == start &&
> > > >  roundup(end, EROFS_BLKSIZ) == end &&
> > > >  (end - start) % EROFS_BLKSIZ == 0
> > > > should be an erofs hole...
> > > >
> > > > But that is minor, I reserve my opinion on this for now...
> > > >
> > > > This patch generally looks good to me, yet I haven't
> > > > played with it till now.
> > > >
> > > > Could you implement a workable RFC patch for kernel side
> > > > as well? Since I'm still busying in XZ library and other
> > > > convert patches for 5.6...
> > > >
> > > > I'd like to hear if some other opinions from Chao and Guifu.
> > > > Since it enhances the on-disk format, we need to think it
> > > > over (especially its future expandability).
> > > >
> > > >
> > > > To Chao and Guifu,
> > > > Could you have some extra time looking at this stuff as well?
> > > >
> > > >
> > > > Thanks,
> > > > Gao Xiang
> > > >
> > > > >
> > > > > Signed-off-by: Pratik Shinde <pratikshinde320@gmail.com>
> > > > > ---
> > > > >  include/erofs/internal.h |   9 ++-
> > > > >  include/erofs_fs.h       |  11 ++++
> > > > >  lib/inode.c              | 153
> > > > ++++++++++++++++++++++++++++++++++++++++-------
> > > > >  3 files changed, 150 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > > > index e13adda..2d7466b 100644
> > > > > --- a/include/erofs/internal.h
> > > > > +++ b/include/erofs/internal.h
> > > > > @@ -63,7 +63,7 @@ struct erofs_sb_info {
> > > > >  extern struct erofs_sb_info sbi;
> > > > >
> > > > >  struct erofs_inode {
> > > > > -     struct list_head i_hash, i_subdirs, i_xattrs;
> > > > > +     struct list_head i_hash, i_subdirs, i_xattrs, i_sparse_extents;
> > > > >
> > > > >       unsigned int i_count;
> > > > >       struct erofs_inode *i_parent;
> > > > > @@ -93,6 +93,7 @@ struct erofs_inode {
> > > > >
> > > > >       unsigned int xattr_isize;
> > > > >       unsigned int extent_isize;
> > > > > +     unsigned int sparse_extent_isize;
> > > > >
> > > > >       erofs_nid_t nid;
> > > > >       struct erofs_buffer_head *bh;
> > > > > @@ -139,5 +140,11 @@ static inline const char *erofs_strerror(int
> > err)
> > > > >       return msg;
> > > > >  }
> > > > >
> > > > > +struct erofs_sparse_extent_node {
> > > > > +     struct list_head next;
> > > > > +     erofs_blk_t lblk;
> > > > > +     erofs_blk_t pblk;
> > > > > +     u32 len;
> > > > > +};
> > > > >  #endif
> > > > >
> > > > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > > > index bcc4f0c..a63e1c6 100644
> > > > > --- a/include/erofs_fs.h
> > > > > +++ b/include/erofs_fs.h
> > > > > @@ -321,5 +321,16 @@ static inline void
> > > > erofs_check_ondisk_layout_definitions(void)
> > > > >                    Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > > > >  }
> > > > >
> > > > > +/* on-disk sparse extent format */
> > > > > +struct erofs_sparse_extent {
> > > > > +     __le32 ee_lblk;
> > > > > +     __le32 ee_pblk;
> > > > > +     __le32 ee_len;
> > > > > +};
> > > > > +
> > > > > +struct erofs_sparse_extent_iheader {
> > > > > +     u32 count;
> > > > > +};
> > > > > +
> > > > >  #endif
> > > > >
> > > > > diff --git a/lib/inode.c b/lib/inode.c
> > > > > index 0e19b11..da20599 100644
> > > > > --- a/lib/inode.c
> > > > > +++ b/lib/inode.c
> > > > > @@ -38,6 +38,97 @@ static unsigned char erofs_type_by_mode[S_IFMT >>
> > > > S_SHIFT] = {
> > > > >
> > > > >  struct list_head inode_hashtable[NR_INODE_HASHTABLE];
> > > > >
> > > > > +
> > > > > +#define IS_HOLE(start, end) (roundup(start, EROFS_BLKSIZ) == start
> > &&
> > > >       \
> > > > > +                          roundup(end, EROFS_BLKSIZ) == end &&
> >  \
> > > > > +                         (end - start) % EROFS_BLKSIZ == 0)
> > > > > +
> > > > > +/**
> > > > > +   read extents of the given file.
> > > > > +   record the data extents and link them into a chain.
> > > > > +   exclude holes present in file.
> > > > > + */
> > > > > +unsigned int erofs_read_sparse_extents(int fd, struct list_head
> > > > *extents)
> > > > > +{
> > > > > +     erofs_blk_t startblk, endblk, datablk;
> > > > > +     unsigned int nholes = 0;
> > > > > +     erofs_off_t data, hole, len = 0, last_data;
> > > > > +     struct erofs_sparse_extent_node *e_data;
> > > > > +
> > > > > +     len = lseek(fd, 0, SEEK_END);
> > > > > +     if (len < 0)
> > > > > +             return -errno;
> > > > > +     if (lseek(fd, 0, SEEK_SET) == -1)
> > > > > +             return -errno;
> > > > > +     data = 0;
> > > > > +     last_data = 0;
> > > > > +     while (data < len) {
> > > > > +             hole = lseek(fd, data, SEEK_HOLE);
> > > > > +             if (hole == len)
> > > > > +                     break;
> > > > > +             data = lseek(fd, hole, SEEK_DATA);
> > > > > +             if (data < 0 || hole > data)
> > > > > +                     return -EINVAL;
> > > > > +             if (IS_HOLE(hole, data)) {
> > > > > +                     startblk = erofs_blknr(hole);
> > > > > +                     datablk = erofs_blknr(last_data);
> > > > > +                     endblk = erofs_blknr(data);
> > > > > +                     last_data = data;
> > > > > +                     e_data = malloc(sizeof(
> > > > > +                                      struct
> > erofs_sparse_extent_node));
> > > > > +                     if (e_data == NULL)
> > > > > +                             return -ENOMEM;
> > > > > +                     e_data->lblk = datablk;
> > > > > +                     e_data->len = (startblk - datablk);
> > > > > +                     list_add_tail(&e_data->next, extents);
> > > > > +                     nholes += (endblk - startblk);
> > > > > +             }
> > > > > +     }
> > > > > +     /* rounddown to exclude tail-end data */
> > > > > +     if (last_data < len && (len - last_data) >= EROFS_BLKSIZ) {
> > > > > +             e_data = malloc(sizeof(struct
> > erofs_sparse_extent_node));
> > > > > +             if (e_data == NULL)
> > > > > +                     return -ENOMEM;
> > > > > +             startblk = erofs_blknr(last_data);
> > > > > +             e_data->lblk = startblk;
> > > > > +             e_data->len = erofs_blknr(rounddown((len - last_data),
> > > > > +                                       EROFS_BLKSIZ));
> > > > > +             list_add_tail(&e_data->next, extents);
> > > > > +     }
> > > > > +     return nholes;
> > > > > +}
> > > > > +
> > > > > +int erofs_write_sparse_extents(struct erofs_inode *inode,
> > erofs_off_t
> > > > off)
> > > > > +{
> > > > > +     struct erofs_sparse_extent_node *e_node;
> > > > > +     struct erofs_sparse_extent_iheader *header;
> > > > > +     char *buf;
> > > > > +     unsigned int p = 0;
> > > > > +     int ret;
> > > > > +
> > > > > +     buf = malloc(inode->sparse_extent_isize);
> > > > > +     if (buf == NULL)
> > > > > +             return -ENOMEM;
> > > > > +     header = (struct erofs_sparse_extent_iheader *) buf;
> > > > > +     header->count = 0;
> > > > > +     p += sizeof(struct erofs_sparse_extent_iheader);
> > > > > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > > > > +             const struct erofs_sparse_extent ee = {
> > > > > +                     .ee_lblk = cpu_to_le32(e_node->lblk),
> > > > > +                     .ee_pblk = cpu_to_le32(e_node->pblk),
> > > > > +                     .ee_len  = cpu_to_le32(e_node->len)
> > > > > +             };
> > > > > +             memcpy(buf + p, &ee, sizeof(struct
> > erofs_sparse_extent));
> > > > > +             p += sizeof(struct erofs_sparse_extent);
> > > > > +             header->count++;
> > > > > +             list_del(&e_node->next);
> > > > > +             free(e_node);
> > > > > +     }
> > > > > +     ret = dev_write(buf, off, inode->sparse_extent_isize);
> > > > > +     free(buf);
> > > > > +     return ret;
> > > > > +}
> > > > > +
> > > > >  void erofs_inode_manager_init(void)
> > > > >  {
> > > > >       unsigned int i;
> > > > > @@ -304,8 +395,9 @@ static bool erofs_file_is_compressible(struct
> > > > erofs_inode *inode)
> > > > >
> > > > >  int erofs_write_file(struct erofs_inode *inode)
> > > > >  {
> > > > > -     unsigned int nblocks, i;
> > > > > +     unsigned int nblocks, i, j, nholes;
> > > > >       int ret, fd;
> > > > > +     struct erofs_sparse_extent_node *e_node;
> > > > >
> > > > >       if (!inode->i_size) {
> > > > >               inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > > > > @@ -322,31 +414,42 @@ int erofs_write_file(struct erofs_inode *inode)
> > > > >       /* fallback to all data uncompressed */
> > > > >       inode->datalayout = EROFS_INODE_FLAT_INLINE;
> > > > >       nblocks = inode->i_size / EROFS_BLKSIZ;
> > > > > -
> > > > > -     ret = __allocate_inode_bh_data(inode, nblocks);
> > > > > -     if (ret)
> > > > > -             return ret;
> > > > > -
> > > > >       fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> > > > >       if (fd < 0)
> > > > >               return -errno;
> > > > > -
> > > > > -     for (i = 0; i < nblocks; ++i) {
> > > > > -             char buf[EROFS_BLKSIZ];
> > > > > -
> > > > > -             ret = read(fd, buf, EROFS_BLKSIZ);
> > > > > -             if (ret != EROFS_BLKSIZ) {
> > > > > -                     if (ret < 0)
> > > > > +     nholes = erofs_read_sparse_extents(fd,
> > &inode->i_sparse_extents);
> > > > > +     if (nholes < 0) {
> > > > > +             close(fd);
> > > > > +             return nholes;
> > > > > +     }
> > > > > +     ret = __allocate_inode_bh_data(inode, nblocks - nholes);
> > > > > +     if (ret) {
> > > > > +             close(fd);
> > > > > +             return ret;
> > > > > +     }
> > > > > +     i = inode->u.i_blkaddr;
> > > > > +     inode->sparse_extent_isize = sizeof(struct
> > > > erofs_sparse_extent_iheader);
> > > > > +     list_for_each_entry(e_node, &inode->i_sparse_extents, next) {
> > > > > +             inode->sparse_extent_isize += sizeof(struct
> > > > erofs_sparse_extent);
> > > > > +             e_node->pblk = i;
> > > > > +             ret = lseek(fd, blknr_to_addr(e_node->lblk), SEEK_SET);
> > > > > +             if (ret < 0)
> > > > > +                     goto fail;
> > > > > +             for (j = 0; j < e_node->len; j++) {
> > > > > +                     char buf[EROFS_BLKSIZ];
> > > > > +                     ret = read(fd, buf, EROFS_BLKSIZ);
> > > > > +                     if (ret != EROFS_BLKSIZ) {
> > > > > +                             if (ret < 0)
> > > > > +                                     goto fail;
> > > > > +                             close(fd);
> > > > > +                             return -EAGAIN;
> > > > > +                     }
> > > > > +                     ret = blk_write(buf, e_node->pblk + j, 1);
> > > > > +                     if (ret)
> > > > >                               goto fail;
> > > > > -                     close(fd);
> > > > > -                     return -EAGAIN;
> > > > >               }
> > > > > -
> > > > > -             ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
> > > > > -             if (ret)
> > > > > -                     goto fail;
> > > > > +             i += e_node->len;
> > > > >       }
> > > > > -
> > > > >       /* read the tail-end data */
> > > > >       inode->idata_size = inode->i_size % EROFS_BLKSIZ;
> > > > >       if (inode->idata_size) {
> > > > > @@ -479,8 +582,14 @@ static bool erofs_bh_flush_write_inode(struct
> > > > erofs_buffer_head *bh)
> > > > >               if (ret)
> > > > >                       return false;
> > > > >               free(inode->compressmeta);
> > > > > +             off += inode->extent_isize;
> > > > >       }
> > > > >
> > > > > +     if (inode->sparse_extent_isize) {
> > > > > +             ret = erofs_write_sparse_extents(inode, off);
> > > > > +             if (ret)
> > > > > +                     return false;
> > > > > +     }
> > > > >       inode->bh = NULL;
> > > > >       erofs_iput(inode);
> > > > >       return erofs_bh_flush_generic_end(bh);
> > > > > @@ -737,10 +846,11 @@ struct erofs_inode *erofs_new_inode(void)
> > > > >
> > > > >       init_list_head(&inode->i_subdirs);
> > > > >       init_list_head(&inode->i_xattrs);
> > > > > +     init_list_head(&inode->i_sparse_extents);
> > > > >
> > > > >       inode->idata_size = 0;
> > > > >       inode->xattr_isize = 0;
> > > > > -     inode->extent_isize = 0;
> > > > > +     inode->sparse_extent_isize = 0;
> > > > >
> > > > >       inode->bh = inode->bh_inline = inode->bh_data = NULL;
> > > > >       inode->idata = NULL;
> > > > > @@ -961,4 +1071,3 @@ struct erofs_inode
> > > > *erofs_mkfs_build_tree_from_path(struct erofs_inode *parent,
> > > > >
> > > > >       return erofs_mkfs_build_tree(inode);
> > > > >  }
> > > > > -
> > > > > --
> > > > > 2.9.3
> > > > >
> > > >
> >

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-02  9:47 [RFCv3] erofs-utils: on-disk extent format for blocks Pratik Shinde
2020-01-02 10:47 ` Gao Xiang
2020-01-02 11:02   ` Pratik Shinde
2020-01-02 13:33     ` Gao Xiang
2020-01-28  6:01       ` Pratik Shinde
2020-01-28  6:37         ` Gao Xiang via Linux-erofs

Linux-EROFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-erofs/0 linux-erofs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-erofs linux-erofs/ https://lore.kernel.org/linux-erofs \
		linux-erofs@lists.ozlabs.org linux-erofs@ozlabs.org
	public-inbox-index linux-erofs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linux-erofs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git