On Wed, Nov 30, 2022 at 04:55:34PM +0800, Yue Hu wrote:
...
> > > @@ -782,22 +846,25 @@ int erofs_write_compressed_file(struct erofs_inode
> > > *inode, int fd)
> > > ctx.head = ctx.tail = 0;
> > > ctx.clusterofs = 0;
> > > ctx.e.length = 0;
> > > - remaining = inode->i_size;
> > > + ctx.e.compressedblks = 0;
> > > + ctx.pclustersize = z_erofs_get_max_pclusterblks(inode) * EROFS_BLKSIZ;
> > > + ctx.remaining = inode->i_size - inode->fragment_size;
> > >
> > > - while (remaining) {
> > > - const u64 readcount = min_t(u64, remaining,
> > > + while (ctx.remaining) {
> > > + const u64 readcount = min_t(u64, ctx.remaining,
> > > sizeof(ctx.queue) - ctx.tail);
> > > + bool fixup = ret < 0;
> >
> > drop this one;
> >
> > >
> > > ret = read(fd, ctx.queue + ctx.tail, readcount);
> > > if (ret != readcount) {
> > > ret = -errno;
> > > goto err_bdrop;
> > > }
> > > - remaining -= readcount;
> > > + ctx.remaining -= readcount;
> > > ctx.tail += readcount;
> > >
> > > - ret = vle_compress_one(&ctx, !remaining);
> >
> > ret == -EAGAIN
>
> Just move 'fixup' in 'ctx'?
Yes.
>
> >
> > > - if (ret)
> > > + ret = vle_compress_one(&ctx, fixup);
> > > + if (ret && ret != -EAGAIN)
> > > goto err_free_idata;
> > > }
> > > DBG_BUGON(ctx.head != ctx.tail);
> > > @@ -807,6 +874,16 @@ int erofs_write_compressed_file(struct erofs_inode
> > > *inode, int fd)
> > > DBG_BUGON(compressed_blocks < !!inode->idata_size);
> > > compressed_blocks -= !!inode->idata_size;
> > >
> > > + if (inode->fragment_size && ctx.e.compressedblks) {
> >
> > why not moving into z_erofs_fragments_dedupe_update()?
>
> I consider this before, it's a bit awkward for me due to non updating case.
>
> Let me reconsider.
What does that mean? "non updating case", could you please write down
some formal words to describe this case?
Thanks,
Gao Xiang
