<p>On Wed, 17 Aug 2022 02:48:54 +0800
<br>Gao Xiang <xiang@kernel.org> wrote:
<br>
<br>> Hi Yue,
<br>>
<br>> I roughly look, some comments below...
<br>
<br>Ok, i will update these in next version.
<br>
<br>>
<br>> On Wed, Aug 03, 2022 at 11:51:28AM +0800, Yue Hu wrote:
<br>> > Add compressed fragments support for erofsfuse.
<br>> >
<br>> > Signed-off-by: Yue Hu <huyue2@coolpad.com>
<br>> > ---
<br>> > include/erofs/internal.h | 8 ++++++++
<br>> > include/erofs_fs.h | 26 ++++++++++++++++++++------
<br>> > lib/data.c | 20 ++++++++++++++++++++
<br>> > lib/super.c | 24 +++++++++++++++++++++++-
<br>> > lib/zmap.c | 26 ++++++++++++++++++++++++++
<br>> > 5 files changed, 97 insertions(+), 7 deletions(-)
<br>> >
<br>> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
<br>> > index 48498fe..5980db7 100644
<br>> > --- a/include/erofs/internal.h
<br>> > +++ b/include/erofs/internal.h
<br>> > @@ -102,6 +102,7 @@ struct erofs_sb_info {
<br>> > u16 devt_slotoff; /* used for mkfs */
<br>> > u16 device_id_mask; /* used for others */
<br>> > };
<br>> > + struct erofs_inode *frags_inode;
<br>>
<br>> I rethought about this feature and the naming.
<br>>
<br>> I think we could name the tail (or the whole file) as a fragment.
<br>>
<br>> But I tend to name the special inode as "packed inode", since
<br>> this special inode can be used as "compressed metadata" as well.
<br>>
<br>> So, just name as "packed_inode"?
<br>>
<br>> > };
<br>> >
<br>> > /* global sbi */
<br>> > @@ -132,6 +133,7 @@ EROFS_FEATURE_FUNCS(big_pcluster, incompat, INCOMPAT_BIG_PCLUSTER)
<br>> > EROFS_FEATURE_FUNCS(chunked_file, incompat, INCOMPAT_CHUNKED_FILE)
<br>> > EROFS_FEATURE_FUNCS(device_table, incompat, INCOMPAT_DEVICE_TABLE)
<br>> > EROFS_FEATURE_FUNCS(ztailpacking, incompat, INCOMPAT_ZTAILPACKING)
<br>> > +EROFS_FEATURE_FUNCS(fragments, incompat, INCOMPAT_FRAGMENTS)
<br>> > EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
<br>> >
<br>> > #define EROFS_I_EA_INITED (1 << 0)
<br>> > @@ -190,6 +192,8 @@ struct erofs_inode {
<br>> > void *eof_tailraw;
<br>> > unsigned int eof_tailrawsize;
<br>> >
<br>> > + erofs_off_t fragmentoff;
<br>>
<br>> move it to the end? or find a better place?
<br>>
<br>> > +
<br>> > union {
<br>> > void *compressmeta;
<br>> > void *chunkindexes;
<br>> > @@ -201,6 +205,7 @@ struct erofs_inode {
<br>> > uint64_t z_tailextent_headlcn;
<br>> > unsigned int z_idataoff;
<br>> > #define z_idata_size idata_size
<br>> > +#define z_fragmentoff fragmentoff
<br>>
<br>> drop this line?
<br>>
<br>> > };
<br>> > };
<br>> > #ifdef WITH_ANDROID
<br>> > @@ -276,6 +281,7 @@ enum {
<br>> > BH_Mapped,
<br>> > BH_Encoded,
<br>> > BH_FullMapped,
<br>> > + BH_Fragments,
<br>>
<br>> BH_Fragment,
<br>>
<br>> > };
<br>> >
<br>> > /* Has a disk mapping */
<br>> > @@ -286,6 +292,8 @@ enum {
<br>> > #define EROFS_MAP_ENCODED (1 << BH_Encoded)
<br>> > /* The length of extent is full */
<br>> > #define EROFS_MAP_FULL_MAPPED (1 << BH_FullMapped)
<br>> > +/* Located in fragments */
<br>> > +#define EROFS_MAP_FRAGMENTS (1 << BH_Fragments)
<br>>
<br>>
<br>> EROFS_MAP_FRAGMENT ?
<br>>
<br>> >
<br>> > struct erofs_map_blocks {
<br>> > char mpage[EROFS_BLKSIZ];
<br>> > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
<br>> > index 08f9761..4e13566 100644
<br>> > --- a/include/erofs_fs.h
<br>> > +++ b/include/erofs_fs.h
<br>> > @@ -25,13 +25,15 @@
<br>> > #define EROFS_FEATURE_INCOMPAT_CHUNKED_FILE 0x00000004
<br>> > #define EROFS_FEATURE_INCOMPAT_DEVICE_TABLE 0x00000008
<br>> > #define EROFS_FEATURE_INCOMPAT_ZTAILPACKING 0x00000010
<br>> > +#define EROFS_FEATURE_INCOMPAT_FRAGMENTS 0x00000020
<br>> > #define EROFS_ALL_FEATURE_INCOMPAT \
<br>> > (EROFS_FEATURE_INCOMPAT_LZ4_0PADDING | \
<br>> > EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
<br>> > EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER | \
<br>> > EROFS_FEATURE_INCOMPAT_CHUNKED_FILE | \
<br>> > EROFS_FEATURE_INCOMPAT_DEVICE_TABLE | \
<br>> > - EROFS_FEATURE_INCOMPAT_ZTAILPACKING)
<br>> > + EROFS_FEATURE_INCOMPAT_ZTAILPACKING | \
<br>> > + EROFS_FEATURE_INCOMPAT_FRAGMENTS)
<br>> >
<br>> > #define EROFS_SB_EXTSLOT_SIZE 16
<br>> >
<br>> > @@ -73,7 +75,9 @@ struct erofs_super_block {
<br>> > } __packed u1;
<br>> > __le16 extra_devices; /* # of devices besides the primary device */
<br>> > __le16 devt_slotoff; /* startoff = devt_slotoff * devt_slotsize */
<br>> > - __u8 reserved2[38];
<br>> > + __u8 reserved[6];
<br>> > + __le64 frags_nid; /* nid of the special fragments inode */
<br>>
<br>> packed_nid; ?
<br>>
<br>> > + __u8 reserved2[24];
<br>> > };
<br>> >
<br>> > /*
<br>> > @@ -294,16 +298,24 @@ struct z_erofs_lzma_cfgs {
<br>> > * bit 1 : HEAD1 big pcluster (0 - off; 1 - on)
<br>> > * bit 2 : HEAD2 big pcluster (0 - off; 1 - on)
<br>> > * bit 3 : tailpacking inline pcluster (0 - off; 1 - on)
<br>> > + * bit 4 : fragment pcluster (0 - off; 1 - on)
<br>> > */
<br>> > #define Z_EROFS_ADVISE_COMPACTED_2B 0x0001
<br>> > #define Z_EROFS_ADVISE_BIG_PCLUSTER_1 0x0002
<br>> > #define Z_EROFS_ADVISE_BIG_PCLUSTER_2 0x0004
<br>> > #define Z_EROFS_ADVISE_INLINE_PCLUSTER 0x0008
<br>> > +#define Z_EROFS_ADVISE_FRAGMENT_PCLUSTER 0x0010
<br>> >
<br>> > struct z_erofs_map_header {
<br>> > - __le16 h_reserved1;
<br>> > - /* record the size of tailpacking data */
<br>> > - __le16 h_idata_size;
<br>> > + union {
<br>> > + /* direct addressing for fragment offset */
<br>> > + __le32 h_fragmentoff;
<br>> > + struct {
<br>> > + __le16 h_reserved1;
<br>> > + /* record the size of tailpacking data */
<br>> > + __le16 h_idata_size;
<br>>
<br>> That is really somewhat a layout mistake when introducing
<br>> ztailpacking feature.
<br>
<br>Oh, we forgot to change this then.
<br>
<br>>
<br>> > + };
<br>> > + };
<br>> > __le16 h_advise;
<br>> > /*
<br>> > * bit 0-3 : algorithm type of head 1 (logical cluster type 01);
<br>> > @@ -312,12 +324,14 @@ struct z_erofs_map_header {
<br>> > __u8 h_algorithmtype;
<br>> > /*
<br>> > * bit 0-2 : logical cluster bits - 12, e.g. 0 for 4096;
<br>> > - * bit 3-7 : reserved.
<br>> > + * bit 3-6 : reserved;
<br>> > + * bit 7 : merge the whole file into fragments or not.
<br>>
<br>> Move the whole file into packed inode or not.
<br>>
<br>> > */
<br>> > __u8 h_clusterbits;
<br>> > };
<br>> >
<br>> > #define Z_EROFS_VLE_LEGACY_HEADER_PADDING 8
<br>> > +#define Z_EROFS_FRAGMENT_INODE_BIT 7
<br>>
<br>> Move this forward, just before "struct z_erofs_map_header"
<br>>
<br>> > /*
<br>> > * Fixed-sized output compression ondisk Logical Extent cluster type:
<br>> > diff --git a/lib/data.c b/lib/data.c
<br>> > index 6bc554d..b9dd07b 100644
<br>> > --- a/lib/data.c
<br>> > +++ b/lib/data.c
<br>> > @@ -275,6 +275,26 @@ static int z_erofs_read_data(struct erofs_inode *inode, char *buffer,
<br>> > continue;
<br>> > }
<br>> >
<br>> > + if (map.m_flags & EROFS_MAP_FRAGMENTS) {
<br>> > + char *out;
<br>> > +
<br>> > + out = malloc(length - skip);
<br>> > + if (!out) {
<br>> > + ret = -ENOMEM;
<br>> > + break;
<br>> > + }
<br>> > + ret = z_erofs_read_data(sbi.frags_inode, out,
<br>> > + length - skip,
<br>> > + inode->z_fragmentoff + skip);
<br>> > + if (ret < 0) {
<br>> > + free(out);
<br>> > + break;
<br>> > + }
<br>> > + memcpy(buffer + end - offset, out, length - skip);
<br>> > + free(out);
<br>> > + continue;
<br>> > + }
<br>> > +
<br>> > if (map.m_plen > bufsize) {
<br>> > bufsize = map.m_plen;
<br>> > raw = realloc(raw, bufsize);
<br>> > diff --git a/lib/super.c b/lib/super.c
<br>> > index b267412..4d3ca00 100644
<br>> > --- a/lib/super.c
<br>> > +++ b/lib/super.c
<br>> > @@ -104,6 +104,21 @@ int erofs_read_superblock(void)
<br>> > sbi.xattr_blkaddr = le32_to_cpu(dsb->xattr_blkaddr);
<br>> > sbi.islotbits = EROFS_ISLOTBITS;
<br>> > sbi.root_nid = le16_to_cpu(dsb->root_nid);
<br>> > + sbi.frags_inode = NULL;
<br>> > + if (erofs_sb_has_fragments()) {
<br>> > + struct erofs_inode *inode;
<br>> > +
<br>> > + inode = calloc(1, sizeof(struct erofs_inode));
<br>> > + if (!inode)
<br>> > + return -ENOMEM;
<br>> > + inode->nid = le64_to_cpu(dsb->frags_nid);
<br>> > + ret = erofs_read_inode_from_disk(inode);
<br>> > + if (ret) {
<br>> > + free(inode);
<br>> > + return ret;
<br>> > + }
<br>> > + sbi.frags_inode = inode;
<br>> > + }
<br>> > sbi.inos = le64_to_cpu(dsb->inos);
<br>> > sbi.checksum = le32_to_cpu(dsb->checksum);
<br>> >
<br>> > @@ -111,11 +126,18 @@ int erofs_read_superblock(void)
<br>> > sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
<br>> >
<br>> > memcpy(&sbi.uuid, dsb->uuid, sizeof(dsb->uuid));
<br>> > - return erofs_init_devices(&sbi, dsb);
<br>> > +
<br>> > + ret = erofs_init_devices(&sbi, dsb);
<br>> > + if (ret && sbi.frags_inode)
<br>> > + free(sbi.frags_inode);
<br>> > + return ret;
<br>> > }
<br>> >
<br>> > void erofs_put_super(void)
<br>> > {
<br>> > if (sbi.devs)
<br>> > free(sbi.devs);
<br>> > +
<br>> > + if (sbi.frags_inode)
<br>> > + free(sbi.frags_inode);
<br>> > }
<br>> > diff --git a/lib/zmap.c b/lib/zmap.c
<br>> > index 95745c5..16267ae 100644
<br>> > --- a/lib/zmap.c
<br>> > +++ b/lib/zmap.c
<br>> > @@ -83,6 +83,20 @@ static int z_erofs_fill_inode_lazy(struct erofs_inode *vi)
<br>> > if (ret < 0)
<br>> > return ret;
<br>> > }
<br>> > + if (vi->z_advise & Z_EROFS_ADVISE_FRAGMENT_PCLUSTER) {
<br>> > + vi->z_fragmentoff = le32_to_cpu(h->h_fragmentoff);
<br>> > +
<br>> > + if (h->h_clusterbits >> Z_EROFS_FRAGMENT_INODE_BIT) {
<br>> > + vi->z_tailextent_headlcn = 0;
<br>> > + } else {
<br>> > + struct erofs_map_blocks map = { .index = UINT_MAX };
<br>> > +
<br>> > + ret = z_erofs_do_map_blocks(vi, &map,
<br>> > + EROFS_GET_BLOCKS_FINDTAIL);
<br>> > + if (ret < 0)
<br>> > + return ret;
<br>> > + }
<br>> > + }
<br>> > vi->flags |= EROFS_I_Z_INITED;
<br>> > return 0;
<br>> > }
<br>> > @@ -546,6 +560,7 @@ static int z_erofs_do_map_blocks(struct erofs_inode *vi,
<br>> > int flags)
<br>> > {
<br>> > bool ztailpacking = vi->z_advise & Z_EROFS_ADVISE_INLINE_PCLUSTER;
<br>> > + bool infrags = vi->z_advise & Z_EROFS_ADVISE_FRAGMENT_PCLUSTER;
<br>>
<br>> inpacked;
<br>>
<br>> Thanks,
<br>> Gao Xiang
<br>>
<br>> > struct z_erofs_maprecorder m = {
<br>> > .inode = vi,
<br>> > .map = map,
<br>> > @@ -609,6 +624,9 @@ static int z_erofs_do_map_blocks(struct erofs_inode *vi,
<br>> > map->m_flags |= EROFS_MAP_META;
<br>> > map->m_pa = vi->z_idataoff;
<br>> > map->m_plen = vi->z_idata_size;
<br>> > + } else if (infrags && m.lcn == vi->z_tailextent_headlcn) {
<br>> > + map->m_flags |= EROFS_MAP_FRAGMENTS;
<br>> > + DBG_BUGON(!map->m_la);
<br>> > } else {
<br>> > map->m_pa = blknr_to_addr(m.pblk);
<br>> > err = z_erofs_get_extent_compressedlen(&m, initial_lcn);
<br>> > @@ -652,6 +670,14 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi,
<br>> > if (err)
<br>> > goto out;
<br>> >
<br>> > + if ((vi->z_advise & Z_EROFS_ADVISE_FRAGMENT_PCLUSTER) &&
<br>> > + !vi->z_tailextent_headlcn) {
<br>> > + map->m_llen = map->m_la + 1;
<br>> > + map->m_la = 0;
<br>> > + map->m_flags = EROFS_MAP_MAPPED | EROFS_MAP_FRAGMENTS;
<br>> > + goto out;
<br>> > + }
<br>> > +
<br>> > err = z_erofs_do_map_blocks(vi, map, flags);
<br>> > out:
<br>> > DBG_BUGON(err < 0 && err != -ENOMEM);
<br>> > --
<br>> > 2.17.1
<br>> ></p><meta data-version="editor_version_1.2.11"/><div data-zone-id="0" data-line-index="0" data-line="true" style="white-space: pre-wrap; margin-top: 4px; margin-bottom: 4px; line-height: 1.6;">------机密：此电子邮件所包含内容为酷派机密内容,并且受到法律的保护。如果您不属于以上电子邮件的目标接收者，您不得细读，使用，传播，散布或复制该电子邮件中的任何信息。如果您已经误收此电子邮件，请您立即通知我们并删除原电子邮件。 CONFIDENTIAL: This e-mail message contains information of Coolpad that is confidential and which is subject to legal privilege.If you are not the intended recipient as indicated above,you must not peruse,use, disseminate,distribute or copy any information contained in this message.If you have received this message in error, please notify us and delete the original message immediately.
</div><div data-zone-id="0" data-line-index="1" data-line="true" style="white-space: pre-wrap; margin-top: 4px; margin-bottom: 4px; line-height: 1.6;">------ 申明：本邮件所现内容，仅作为我们之间就合作的事宜进行的交流、沟通、洽谈、商议，不作为协议或承诺，一切协议及承诺必须以书面文本盖章为准。 DECLARATION：All contents of this E-mail ,are only regarded as the cooperation we have had between the exchanges, communication, negotiation and deliberation, not as a agreement or promise. All contracts and commitments must be sealed shall prevail.
</div>