On Wed, 17 Aug 2022 02:48:54 +0800 Gao Xiang <[email protected]> wrote:
> Hi Yue, > > I roughly look, some comments below... Ok, i will update these in next version. > > On Wed, Aug 03, 2022 at 11:51:28AM +0800, Yue Hu wrote: > > Add compressed fragments support for erofsfuse. > > > > Signed-off-by: Yue Hu <[email protected]> > > --- > > include/erofs/internal.h | 8 ++++++++ > > include/erofs_fs.h | 26 ++++++++++++++++++++------ > > lib/data.c | 20 ++++++++++++++++++++ > > lib/super.c | 24 +++++++++++++++++++++++- > > lib/zmap.c | 26 ++++++++++++++++++++++++++ > > 5 files changed, 97 insertions(+), 7 deletions(-) > > > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h > > index 48498fe..5980db7 100644 > > --- a/include/erofs/internal.h > > +++ b/include/erofs/internal.h > > @@ -102,6 +102,7 @@ struct erofs_sb_info { > > u16 devt_slotoff; /* used for mkfs */ > > u16 device_id_mask; /* used for others */ > > }; > > + struct erofs_inode *frags_inode; > > I rethought about this feature and the naming. > > I think we could name the tail (or the whole file) as a fragment. > > But I tend to name the special inode as "packed inode", since > this special inode can be used as "compressed metadata" as well. > > So, just name as "packed_inode"? > > > }; > > > > /* global sbi */ > > @@ -132,6 +133,7 @@ EROFS_FEATURE_FUNCS(big_pcluster, incompat, > > INCOMPAT_BIG_PCLUSTER) > > EROFS_FEATURE_FUNCS(chunked_file, incompat, INCOMPAT_CHUNKED_FILE) > > EROFS_FEATURE_FUNCS(device_table, incompat, INCOMPAT_DEVICE_TABLE) > > EROFS_FEATURE_FUNCS(ztailpacking, incompat, INCOMPAT_ZTAILPACKING) > > +EROFS_FEATURE_FUNCS(fragments, incompat, INCOMPAT_FRAGMENTS) > > EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM) > > > > #define EROFS_I_EA_INITED (1 << 0) > > @@ -190,6 +192,8 @@ struct erofs_inode { > > void *eof_tailraw; > > unsigned int eof_tailrawsize; > > > > + erofs_off_t fragmentoff; > > move it to the end? or find a better place? > > > + > > union { > > void *compressmeta; > > void *chunkindexes; > > @@ -201,6 +205,7 @@ struct erofs_inode { > > uint64_t z_tailextent_headlcn; > > unsigned int z_idataoff; > > #define z_idata_size idata_size > > +#define z_fragmentoff fragmentoff > > drop this line? > > > }; > > }; > > #ifdef WITH_ANDROID > > @@ -276,6 +281,7 @@ enum { > > BH_Mapped, > > BH_Encoded, > > BH_FullMapped, > > + BH_Fragments, > > BH_Fragment, > > > }; > > > > /* Has a disk mapping */ > > @@ -286,6 +292,8 @@ enum { > > #define EROFS_MAP_ENCODED (1 << BH_Encoded) > > /* The length of extent is full */ > > #define EROFS_MAP_FULL_MAPPED (1 << BH_FullMapped) > > +/* Located in fragments */ > > +#define EROFS_MAP_FRAGMENTS (1 << BH_Fragments) > > > EROFS_MAP_FRAGMENT ? > > > > > struct erofs_map_blocks { > > char mpage[EROFS_BLKSIZ]; > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h > > index 08f9761..4e13566 100644 > > --- a/include/erofs_fs.h > > +++ b/include/erofs_fs.h > > @@ -25,13 +25,15 @@ > > #define EROFS_FEATURE_INCOMPAT_CHUNKED_FILE 0x00000004 > > #define EROFS_FEATURE_INCOMPAT_DEVICE_TABLE 0x00000008 > > #define EROFS_FEATURE_INCOMPAT_ZTAILPACKING 0x00000010 > > +#define EROFS_FEATURE_INCOMPAT_FRAGMENTS 0x00000020 > > #define EROFS_ALL_FEATURE_INCOMPAT \ > > (EROFS_FEATURE_INCOMPAT_LZ4_0PADDING | \ > > EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \ > > EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER | \ > > EROFS_FEATURE_INCOMPAT_CHUNKED_FILE | \ > > EROFS_FEATURE_INCOMPAT_DEVICE_TABLE | \ > > - EROFS_FEATURE_INCOMPAT_ZTAILPACKING) > > + EROFS_FEATURE_INCOMPAT_ZTAILPACKING | \ > > + EROFS_FEATURE_INCOMPAT_FRAGMENTS) > > > > #define EROFS_SB_EXTSLOT_SIZE 16 > > > > @@ -73,7 +75,9 @@ struct erofs_super_block { > > } __packed u1; > > __le16 extra_devices; /* # of devices besides the primary device */ > > __le16 devt_slotoff; /* startoff = devt_slotoff * devt_slotsize */ > > - __u8 reserved2[38]; > > + __u8 reserved[6]; > > + __le64 frags_nid; /* nid of the special fragments inode */ > > packed_nid; ? > > > + __u8 reserved2[24]; > > }; > > > > /* > > @@ -294,16 +298,24 @@ struct z_erofs_lzma_cfgs { > > * bit 1 : HEAD1 big pcluster (0 - off; 1 - on) > > * bit 2 : HEAD2 big pcluster (0 - off; 1 - on) > > * bit 3 : tailpacking inline pcluster (0 - off; 1 - on) > > + * bit 4 : fragment pcluster (0 - off; 1 - on) > > */ > > #define Z_EROFS_ADVISE_COMPACTED_2B 0x0001 > > #define Z_EROFS_ADVISE_BIG_PCLUSTER_1 0x0002 > > #define Z_EROFS_ADVISE_BIG_PCLUSTER_2 0x0004 > > #define Z_EROFS_ADVISE_INLINE_PCLUSTER 0x0008 > > +#define Z_EROFS_ADVISE_FRAGMENT_PCLUSTER 0x0010 > > > > struct z_erofs_map_header { > > - __le16 h_reserved1; > > - /* record the size of tailpacking data */ > > - __le16 h_idata_size; > > + union { > > + /* direct addressing for fragment offset */ > > + __le32 h_fragmentoff; > > + struct { > > + __le16 h_reserved1; > > + /* record the size of tailpacking data */ > > + __le16 h_idata_size; > > That is really somewhat a layout mistake when introducing > ztailpacking feature. Oh, we forgot to change this then. > > > + }; > > + }; > > __le16 h_advise; > > /* > > * bit 0-3 : algorithm type of head 1 (logical cluster type 01); > > @@ -312,12 +324,14 @@ struct z_erofs_map_header { > > __u8 h_algorithmtype; > > /* > > * bit 0-2 : logical cluster bits - 12, e.g. 0 for 4096; > > - * bit 3-7 : reserved. > > + * bit 3-6 : reserved; > > + * bit 7 : merge the whole file into fragments or not. > > Move the whole file into packed inode or not. > > > */ > > __u8 h_clusterbits; > > }; > > > > #define Z_EROFS_VLE_LEGACY_HEADER_PADDING 8 > > +#define Z_EROFS_FRAGMENT_INODE_BIT 7 > > Move this forward, just before "struct z_erofs_map_header" > > > /* > > * Fixed-sized output compression ondisk Logical Extent cluster type: > > diff --git a/lib/data.c b/lib/data.c > > index 6bc554d..b9dd07b 100644 > > --- a/lib/data.c > > +++ b/lib/data.c > > @@ -275,6 +275,26 @@ static int z_erofs_read_data(struct erofs_inode > > *inode, char *buffer, > > continue; > > } > > > > + if (map.m_flags & EROFS_MAP_FRAGMENTS) { > > + char *out; > > + > > + out = malloc(length - skip); > > + if (!out) { > > + ret = -ENOMEM; > > + break; > > + } > > + ret = z_erofs_read_data(sbi.frags_inode, out, > > + length - skip, > > + inode->z_fragmentoff + skip); > > + if (ret < 0) { > > + free(out); > > + break; > > + } > > + memcpy(buffer + end - offset, out, length - skip); > > + free(out); > > + continue; > > + } > > + > > if (map.m_plen > bufsize) { > > bufsize = map.m_plen; > > raw = realloc(raw, bufsize); > > diff --git a/lib/super.c b/lib/super.c > > index b267412..4d3ca00 100644 > > --- a/lib/super.c > > +++ b/lib/super.c > > @@ -104,6 +104,21 @@ int erofs_read_superblock(void) > > sbi.xattr_blkaddr = le32_to_cpu(dsb->xattr_blkaddr); > > sbi.islotbits = EROFS_ISLOTBITS; > > sbi.root_nid = le16_to_cpu(dsb->root_nid); > > + sbi.frags_inode = NULL; > > + if (erofs_sb_has_fragments()) { > > + struct erofs_inode *inode; > > + > > + inode = calloc(1, sizeof(struct erofs_inode)); > > + if (!inode) > > + return -ENOMEM; > > + inode->nid = le64_to_cpu(dsb->frags_nid); > > + ret = erofs_read_inode_from_disk(inode); > > + if (ret) { > > + free(inode); > > + return ret; > > + } > > + sbi.frags_inode = inode; > > + } > > sbi.inos = le64_to_cpu(dsb->inos); > > sbi.checksum = le32_to_cpu(dsb->checksum); > > > > @@ -111,11 +126,18 @@ int erofs_read_superblock(void) > > sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec); > > > > memcpy(&sbi.uuid, dsb->uuid, sizeof(dsb->uuid)); > > - return erofs_init_devices(&sbi, dsb); > > + > > + ret = erofs_init_devices(&sbi, dsb); > > + if (ret && sbi.frags_inode) > > + free(sbi.frags_inode); > > + return ret; > > } > > > > void erofs_put_super(void) > > { > > if (sbi.devs) > > free(sbi.devs); > > + > > + if (sbi.frags_inode) > > + free(sbi.frags_inode); > > } > > diff --git a/lib/zmap.c b/lib/zmap.c > > index 95745c5..16267ae 100644 > > --- a/lib/zmap.c > > +++ b/lib/zmap.c > > @@ -83,6 +83,20 @@ static int z_erofs_fill_inode_lazy(struct erofs_inode > > *vi) > > if (ret < 0) > > return ret; > > } > > + if (vi->z_advise & Z_EROFS_ADVISE_FRAGMENT_PCLUSTER) { > > + vi->z_fragmentoff = le32_to_cpu(h->h_fragmentoff); > > + > > + if (h->h_clusterbits >> Z_EROFS_FRAGMENT_INODE_BIT) { > > + vi->z_tailextent_headlcn = 0; > > + } else { > > + struct erofs_map_blocks map = { .index = UINT_MAX }; > > + > > + ret = z_erofs_do_map_blocks(vi, &map, > > + EROFS_GET_BLOCKS_FINDTAIL); > > + if (ret < 0) > > + return ret; > > + } > > + } > > vi->flags |= EROFS_I_Z_INITED; > > return 0; > > } > > @@ -546,6 +560,7 @@ static int z_erofs_do_map_blocks(struct erofs_inode *vi, > > int flags) > > { > > bool ztailpacking = vi->z_advise & Z_EROFS_ADVISE_INLINE_PCLUSTER; > > + bool infrags = vi->z_advise & Z_EROFS_ADVISE_FRAGMENT_PCLUSTER; > > inpacked; > > Thanks, > Gao Xiang > > > struct z_erofs_maprecorder m = { > > .inode = vi, > > .map = map, > > @@ -609,6 +624,9 @@ static int z_erofs_do_map_blocks(struct erofs_inode *vi, > > map->m_flags |= EROFS_MAP_META; > > map->m_pa = vi->z_idataoff; > > map->m_plen = vi->z_idata_size; > > + } else if (infrags && m.lcn == vi->z_tailextent_headlcn) { > > + map->m_flags |= EROFS_MAP_FRAGMENTS; > > + DBG_BUGON(!map->m_la); > > } else { > > map->m_pa = blknr_to_addr(m.pblk); > > err = z_erofs_get_extent_compressedlen(&m, initial_lcn); > > @@ -652,6 +670,14 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi, > > if (err) > > goto out; > > > > + if ((vi->z_advise & Z_EROFS_ADVISE_FRAGMENT_PCLUSTER) && > > + !vi->z_tailextent_headlcn) { > > + map->m_llen = map->m_la + 1; > > + map->m_la = 0; > > + map->m_flags = EROFS_MAP_MAPPED | EROFS_MAP_FRAGMENTS; > > + goto out; > > + } > > + > > err = z_erofs_do_map_blocks(vi, map, flags); > > out: > > DBG_BUGON(err < 0 && err != -ENOMEM); > > -- > > 2.17.1 > >
