On Wed, Apr 23, 2025 at 6:50 PM Hongbo Li <lihongb...@huawei.com> wrote:
>
> Hi Sandeep,
> The consecutive chunks will be merged if possible, but after commit
> 545988a65131 ("erofs-utils: lib: Fix calculation of minextblks when
> working with sparse files"), the @minextblks will be updated into a
> smaller value even the chunks are consecutive by blobchunks.c:379. I
> think maybe the last operation that updates @minextblks is unnecessary,
> since this value would have already been adjusted earlier when handling
> discontinuous chunks. Likes:
>
> ```
> --- a/lib/blobchunk.c
> +++ b/lib/blobchunk.c
> @@ -376,7 +376,6 @@ int erofs_blob_write_chunked_file(struct erofs_inode
> *inode, int fd,
> *(void **)idx++ = chunk;
> lastch = chunk;
> }
> - erofs_update_minextblks(sbi, interval_start, pos, &minextblks);
> inode->datalayout = EROFS_INODE_CHUNK_BASED;
> free(chunkdata);
> return erofs_blob_mergechunks(inode, chunkbits,
>
> ```
> This way can reduces the chunk index array's size. And what about your
> opinion?
>
> Thanks,
> Hongbo
Hi Hongbo,
I think the last call is necessary to handle the tail end which is not
handled in the for loop. But I understand that if the file is
contiguous then the last call can reduce minextblks.
Does the below patch address your concern to conditionally call the
last erofs_update_minextblks()?
Thanks,
Sandeep.
diff --git a/lib/blobchunk.c b/lib/blobchunk.c
index de9150f..47fe923 100644
--- a/lib/blobchunk.c
+++ b/lib/blobchunk.c
@@ -303,6 +303,7 @@ int erofs_blob_write_chunked_file(struct
erofs_inode *inode, int fd,
lastch = NULL;
minextblks = BLK_ROUND_UP(sbi, inode->i_size);
interval_start = 0;
+ bool is_contiguous = true;
for (pos = 0; pos < inode->i_size; pos += len) {
#ifdef SEEK_DATA
@@ -332,6 +333,7 @@ int erofs_blob_write_chunked_file(struct
erofs_inode *inode, int fd,
erofs_update_minextblks(sbi, interval_start,
pos, &minextblks);
interval_start = pos;
+ is_contiguous = false;
}
do {
*(void **)idx++ = &erofs_holechunk;
@@ -365,7 +367,8 @@ int erofs_blob_write_chunked_file(struct
erofs_inode *inode, int fd,
*(void **)idx++ = chunk;
lastch = chunk;
}
- erofs_update_minextblks(sbi, interval_start, pos, &minextblks);
+ if (!is_contiguous)
+ erofs_update_minextblks(sbi, interval_start, pos, &minextblks);
inode->datalayout = EROFS_INODE_CHUNK_BASED;
free(chunkdata);
return erofs_blob_mergechunks(inode, chunkbits,
