[PATCH] erofs-utils: mkfs: skip the redundant write for ztailpacking block

[Yifan Zhao]

On 4/18/24 9:09 AM, Noboru Asai wrote:
> In this patch, the value of blkaddr in z_erofs_lcluster_index
> corresponding to the ztailpacking block in the extent list
> is invalid value. It looks that the linux kernel doesn't refer to this
> value, but what value is correct?
> 0 or -1 (EROFS_NULL_ADDR) or don't care?

For the read side, AFAIK the kernel (and also the user lib of 
erofs-utils) will turn to the meta for inlined data

for read offset beyond the last data block if ztailpacking is enabled, 
see `erofs_map_blocks_flatmode`.

So I believe the blkaddr recorded in z_erofs_lcluster_index is 
irrelevant and we don't need to care about it.


For the mkfs side I think things could be polished according to Gao's 
review opinion.


Thanks,

Yifan Zhao

>
> 2024年4月17日(水) 23:43 Yifan Zhao <zhaoyifan at sjtu.edu.cn>:
>> z_erofs_merge_segment() doesn't consider the ztailpacking block in the
>> extent list and unnecessarily writes it back to the disk. This patch
>> fixes this issue by introducing a new `inlined` field in the struct
>> `z_erofs_inmem_extent`.
>>
>> Signed-off-by: Yifan Zhao <zhaoyifan at sjtu.edu.cn>
>> ---
>>   include/erofs/dedupe.h | 2 +-
>>   lib/compress.c         | 8 ++++++++
>>   lib/dedupe.c           | 1 +
>>   3 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/erofs/dedupe.h b/include/erofs/dedupe.h
>> index 153bd4c..4cbfb2c 100644
>> --- a/include/erofs/dedupe.h
>> +++ b/include/erofs/dedupe.h
>> @@ -16,7 +16,7 @@ struct z_erofs_inmem_extent {
>>          erofs_blk_t blkaddr;
>>          unsigned int compressedblks;
>>          unsigned int length;
>> -       bool raw, partial;
>> +       bool raw, partial, inlined;
>>   };
>>
>>   struct z_erofs_dedupe_ctx {
>> diff --git a/lib/compress.c b/lib/compress.c
>> index 74c5707..41628e7 100644
>> --- a/lib/compress.c
>> +++ b/lib/compress.c
>> @@ -572,6 +572,7 @@ nocompression:
>>                   */
>>                  e->compressedblks = 1;
>>                  e->raw = true;
>> +               e->inlined = false;
>>          } else if (may_packing && len == e->length &&
>>                     compressedsize < ctx->pclustersize &&
>>                     (!inode->fragment_size || ictx->fix_dedupedfrag)) {
>> @@ -582,6 +583,7 @@ frag_packing:
>>                          return ret;
>>                  e->compressedblks = 0; /* indicate a fragment */
>>                  e->raw = false;
>> +               e->inlined = false;
>>                  ictx->fragemitted = true;
>>          /* tailpcluster should be less than 1 block */
>>          } else if (may_inline && len == e->length && compressedsize < blksz) {
>> @@ -600,6 +602,7 @@ frag_packing:
>>                          return ret;
>>                  e->compressedblks = 1;
>>                  e->raw = false;
>> +               e->inlined = true;
>>          } else {
>>                  unsigned int tailused, padding;
>>
>> @@ -652,6 +655,7 @@ frag_packing:
>>                                  return ret;
>>                  }
>>                  e->raw = false;
>> +               e->inlined = false;
>>                  may_inline = false;
>>                  may_packing = false;
>>          }
>> @@ -1151,6 +1155,9 @@ int z_erofs_merge_segment(struct z_erofs_compress_ictx *ictx,
>>                  ei->e.blkaddr = sctx->blkaddr;
>>                  sctx->blkaddr += ei->e.compressedblks;
>>
>> +               if (ei->e.inlined)
>> +                       continue;
>> +
>>                  ret2 = blk_write(sbi, sctx->membuf + blkoff * erofs_blksiz(sbi),
>>                                   ei->e.blkaddr, ei->e.compressedblks);
>>                  blkoff += ei->e.compressedblks;
>> @@ -1374,6 +1381,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode, int fd, u64 fpos)
>>                          .compressedblks = 0,
>>                          .raw = false,
>>                          .partial = false,
>> +                       .inlined = false,
>>                          .blkaddr = sctx.blkaddr,
>>                  };
>>                  init_list_head(&ei->list);
>> diff --git a/lib/dedupe.c b/lib/dedupe.c
>> index 19a1c8d..aaaccb5 100644
>> --- a/lib/dedupe.c
>> +++ b/lib/dedupe.c
>> @@ -138,6 +138,7 @@ int z_erofs_dedupe_match(struct z_erofs_dedupe_ctx *ctx)
>>                  ctx->e.partial = e->partial ||
>>                          (window_size + extra < e->original_length);
>>                  ctx->e.raw = e->raw;
>> +               ctx->e.inlined = false;
>>                  ctx->e.blkaddr = e->compressed_blkaddr;
>>                  ctx->e.compressedblks = e->compressed_blks;
>>                  return 0;
>> --
>> 2.44.0
>>