Re: [PATCH 1/5] erofs-utils: lib: track packed inode position explicitly

[Yifan Zhao <stopire@gmail.com>]

On 3/11/26 08:45, Gao Xiang wrote:
> Track the packed inode position in memory instead of querying the
> file offset. Use positional writes for fragment data and seek
> explicitly before the sendfile fallback.
>
> Factor out __erofs_io_pwrite() and fix its short-write handling.
>
> Assisted-by: ChatGPT:gpt-6.1-sol
> Signed-off-by: Gao Xiang <xiang.gao@docker.com>
> ---
>   include/erofs/io.h |  2 +-
>   lib/fragments.c    | 44 +++++++++++++++++++++-----------------------
>   lib/io.c           | 29 ++++++++++++++---------------
>   3 files changed, 36 insertions(+), 39 deletions(-)
>
> diff --git a/include/erofs/io.h b/include/erofs/io.h
> index 96309fd..f3d4899 100644
> --- a/include/erofs/io.h
> +++ b/include/erofs/io.h
> @@ -68,7 +68,7 @@ struct erofs_vfile {
>   	};
>   };
>   
> -ssize_t __erofs_io_write(int fd, const void *buf, size_t len);
> +ssize_t __erofs_io_pwrite(int fd, const void *buf, size_t len, u64 pos);
>   int __erofs_0write(int fd, size_t len);
>   
>   int erofs_io_fstat(struct erofs_vfile *vf, struct stat *buf);
> diff --git a/lib/fragments.c b/lib/fragments.c
> index f137e41..bb9ef72 100644
> --- a/lib/fragments.c
> +++ b/lib/fragments.c
> @@ -49,6 +49,7 @@ struct erofs_packed_inode {
>   	unsigned long *uptodate;
>   	erofs_mutex_t mutex;
>   	u64 uptodate_bits;
> +	erofs_off_t pos;

erofs_xattr_flush_name_prefixes() also manipulates the packed inode's fd 
offset.


     if (bh) {
         bh->op = &erofs_drop_directly_bhops;
         err = erofs_bh_balloon(bh, offset - start);
         if (err < 0)
             return err;
         bh->op = &erofs_drop_directly_bhops;
         erofs_bdrop(bh, false);
     } else {
         DBG_BUGON(bmgr);
         if (lseek(vf->fd, offset, SEEK_SET) < 0) // maybe a packedinode
             return -errno;
     }


Introducing a separate pos field would leave the two out of sync; how 
should this be handled?


Thanks,

Yifan Zhao

>   };
>   
>   const char *erofs_frags_packedname = "packed_file";
> @@ -245,9 +246,7 @@ int erofs_pack_file_from_fd(struct erofs_inode *inode,
>   	if (__erofs_unlikely(!inode->i_size))
>   		return 0;
>   
> -	offset = lseek(epi->fd, 0, SEEK_CUR);
> -	if (offset < 0)
> -		return -errno;
> +	offset = epi->pos;
>   
>   	if (vf->ops)
>   		memblock = NULL;
> @@ -260,20 +259,24 @@ int erofs_pack_file_from_fd(struct erofs_inode *inode,
>   		bool noseek = vf->ops && !vf->ops->pread;
>   		off_t pos = fpos;
>   
> +		memblock = NULL;
> +		rc = lseek(epi->fd, offset, SEEK_SET);
> +		if (rc < 0) {
> +			rc = -errno;
> +			goto out;
> +		}
>   		do {
>   			sz = min_t(u64, remaining, UINT_MAX);
>   			rc = erofs_io_sendfile(&vout, vf,
>   					       noseek ? NULL : &pos, sz);
> -			if (rc <= 0)
> -				break;
> +			if (rc <= 0) {
> +				if (!rc)
> +					rc = -EIO;
> +				goto out;
> +			}
>   			remaining -= rc;
>   		} while (remaining);
>   
> -		if (remaining && rc >= 0) {
> -			rc = -EIO;
> -			goto out;
> -		}
> -
>   		sz = min_t(u64, inode->i_size, EROFS_FRAGMENT_INMEM_SZ_MAX);
>   		memblock = malloc(sz);
>   		if (!memblock) {
> @@ -291,7 +294,7 @@ int erofs_pack_file_from_fd(struct erofs_inode *inode,
>   			goto out;
>   		}
>   	} else {
> -		rc = __erofs_io_write(epi->fd, memblock, inode->i_size);
> +		rc = __erofs_io_pwrite(epi->fd, memblock, inode->i_size, offset);
>   		if (rc != inode->i_size) {
>   			if (rc >= 0)
>   				rc = -EIO;
> @@ -299,6 +302,7 @@ int erofs_pack_file_from_fd(struct erofs_inode *inode,
>   		}
>   	}
>   
> +	epi->pos = offset + inode->i_size;
>   	rc = erofs_fragment_pack(inode, memblock, offset, inode->i_size,
>   				 tofh, onheap);
>   out:
> @@ -332,17 +336,16 @@ int erofs_fragment_commit(struct erofs_inode *inode, u32 tofh)
>   		return 0;
>   	}
>   
> -	offset = lseek(epi->fd, 0, SEEK_CUR);
> -	if (offset < 0)
> -		return -errno;
> +	offset = epi->pos;
>   
> -	ret = write(epi->fd, fi->data, fi->length);
> +	ret = __erofs_io_pwrite(epi->fd, fi->data, fi->length, offset);
>   	if (ret != fi->length) {
>   		if (ret < 0)
>   			return -errno;
>   		return -EIO;
>   	}
>   	offset += fi->length;
> +	epi->pos = offset;
>   
>   	if (!list_empty(&fi->list)) {
>   		struct erofs_fragment_bucket *bk = &epi->bks[FRAGMENT_HASH(tofh)];
> @@ -380,7 +383,7 @@ int erofs_flush_packed_inode(struct erofs_importer *im)
>   	if (!epi || !erofs_sb_has_fragments(sbi))
>   		return 0;
>   
> -	if (lseek(epi->fd, 0, SEEK_CUR) <= 0)
> +	if (!epi->pos)
>   		return 0;
>   
>   	erofs_update_progressinfo("Processing packed data ...");
> @@ -460,7 +463,6 @@ int erofs_packedfile_init(struct erofs_sb_info *sbi, bool fragments_mkfs)
>   			.sbi = sbi,
>   			.nid = sbi->packed_nid,
>   		};
> -		s64 offset;
>   
>   		err = erofs_read_inode_from_disk(&ei);
>   		if (err) {
> @@ -469,11 +471,7 @@ int erofs_packedfile_init(struct erofs_sb_info *sbi, bool fragments_mkfs)
>   			goto err_out;
>   		}
>   
> -		offset = lseek(epi->fd, ei.i_size, SEEK_SET);
> -		if (offset < 0) {
> -			err = -errno;
> -			goto err_out;
> -		}
> +		epi->pos = ei.i_size;
>   		epi->uptodate_bits = round_up(BLK_ROUND_UP(sbi, ei.i_size),
>   					      sizeof(epi->uptodate) * 8);
>   		epi->uptodate = calloc(1, epi->uptodate_bits >> 3);
> @@ -548,7 +546,7 @@ static void *erofs_packedfile_preload(struct erofs_inode *pi,
>   	if (err)
>   		goto err_out;
>   
> -	err = pwrite(epi->fd, buffer, map->m_llen, map->m_la);
> +	err = __erofs_io_pwrite(epi->fd, buffer, map->m_llen, map->m_la);
>   	if (err < 0) {
>   		err = -errno;
>   		if (err == -ENOSPC) {
> diff --git a/lib/io.c b/lib/io.c
> index 3333170..14253af 100644
> --- a/lib/io.c
> +++ b/lib/io.c
> @@ -26,7 +26,7 @@
>   #define EROFS_MODNAME	"erofs_io"
>   #include "erofs/print.h"
>   
> -ssize_t __erofs_io_write(int fd, const void *buf, size_t len)
> +static ssize_t __erofs_io_write(int fd, const void *buf, size_t len)
>   {
>   	ssize_t ret, written = 0;
>   
> @@ -61,24 +61,12 @@ int erofs_io_fstat(struct erofs_vfile *vf, struct stat *buf)
>   	return fstat(vf->fd, buf);
>   }
>   
> -ssize_t erofs_io_pwrite(struct erofs_vfile *vf, const void *buf,
> -			u64 pos, size_t len)
> +ssize_t __erofs_io_pwrite(int fd, const void *buf, size_t len, u64 pos)
>   {
>   	ssize_t ret, written = 0;
>   
> -	if (__erofs_unlikely(cfg.c_dry_run))
> -		return 0;
> -
> -	if (vf->ops)
> -		return vf->ops->pwrite(vf, buf, pos, len);
> -
> -	pos += vf->offset;
>   	do {
> -#ifdef HAVE_PWRITE64
> -		ret = pwrite64(vf->fd, buf, len, (off64_t)pos);
> -#else
> -		ret = pwrite(vf->fd, buf, len, (off_t)pos);
> -#endif
> +		ret = pwrite(fd, buf, len - written, (off_t)pos);
>   		if (ret <= 0) {
>   			if (!ret)
>   				break;
> @@ -96,6 +84,17 @@ ssize_t erofs_io_pwrite(struct erofs_vfile *vf, const void *buf,
>   	return written;
>   }
>   
> +ssize_t erofs_io_pwrite(struct erofs_vfile *vf, const void *buf,
> +			u64 pos, size_t len)
> +{
> +	if (__erofs_unlikely(cfg.c_dry_run))
> +		return 0;
> +
> +	if (vf->ops)
> +		return vf->ops->pwrite(vf, buf, pos, len);
> +	return __erofs_io_pwrite(vf->fd, buf, len, pos + vf->offset);
> +}
> +
>   ssize_t erofs_io_pwritev(struct erofs_vfile *vf, const struct iovec *iov,
>   			 int iovcnt, u64 pos)
>   {