2022-08-12 06:23:44

by Yue Hu

[permalink] [raw]
Subject: [PATCH] erofs: avoid the potentially wrong m_plen for big pcluster

Actually, 'compressedlcs' stores compressed block count rather than
lcluster count. Therefore, the number of bits for shifting the count
should be 'LOG_BLOCK_SIZE' rather than 'lclusterbits' although current
lcluster size is 4K. The value of 'm_plen' will be wrong once we enable
the non 4K-sized lcluster.

Signed-off-by: Yue Hu <[email protected]>
---
fs/erofs/zmap.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 572f0b8151ba..d58549ca1df9 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -141,7 +141,7 @@ struct z_erofs_maprecorder {
u8 type, headtype;
u16 clusterofs;
u16 delta[2];
- erofs_blk_t pblk, compressedlcs;
+ erofs_blk_t pblk, compressedblks;
erofs_off_t nextpackoff;
};

@@ -192,7 +192,7 @@ static int legacy_load_cluster_from_disk(struct z_erofs_maprecorder *m,
DBG_BUGON(1);
return -EFSCORRUPTED;
}
- m->compressedlcs = m->delta[0] &
+ m->compressedblks = m->delta[0] &
~Z_EROFS_VLE_DI_D0_CBLKCNT;
m->delta[0] = 1;
}
@@ -293,7 +293,7 @@ static int unpack_compacted_index(struct z_erofs_maprecorder *m,
DBG_BUGON(1);
return -EFSCORRUPTED;
}
- m->compressedlcs = lo & ~Z_EROFS_VLE_DI_D0_CBLKCNT;
+ m->compressedblks = lo & ~Z_EROFS_VLE_DI_D0_CBLKCNT;
m->delta[0] = 1;
return 0;
} else if (i + 1 != (int)vcnt) {
@@ -497,7 +497,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
return 0;
}
lcn = m->lcn + 1;
- if (m->compressedlcs)
+ if (m->compressedblks)
goto out;

err = z_erofs_load_cluster_from_disk(m, lcn, false);
@@ -506,7 +506,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,

/*
* If the 1st NONHEAD lcluster has already been handled initially w/o
- * valid compressedlcs, which means at least it mustn't be CBLKCNT, or
+ * valid compressedblks, which means at least it mustn't be CBLKCNT, or
* an internal implemenatation error is detected.
*
* The following code can also handle it properly anyway, but let's
@@ -523,12 +523,12 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
* if the 1st NONHEAD lcluster is actually PLAIN or HEAD type
* rather than CBLKCNT, it's a 1 lcluster-sized pcluster.
*/
- m->compressedlcs = 1;
+ m->compressedblks = 1 << (lclusterbits - LOG_BLOCK_SIZE);
break;
case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
if (m->delta[0] != 1)
goto err_bonus_cblkcnt;
- if (m->compressedlcs)
+ if (m->compressedblks)
break;
fallthrough;
default:
@@ -539,7 +539,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
return -EFSCORRUPTED;
}
out:
- map->m_plen = (u64)m->compressedlcs << lclusterbits;
+ map->m_plen = (u64)m->compressedblks << LOG_BLOCK_SIZE;
return 0;
err_bonus_cblkcnt:
erofs_err(m->inode->i_sb,
--
2.17.1


2022-08-12 07:53:40

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH] erofs: avoid the potentially wrong m_plen for big pcluster

Hi Yue,

On Fri, Aug 12, 2022 at 02:01:50PM +0800, Yue Hu wrote:
> Actually, 'compressedlcs' stores compressed block count rather than
> lcluster count. Therefore, the number of bits for shifting the count
> should be 'LOG_BLOCK_SIZE' rather than 'lclusterbits' although current
> lcluster size is 4K. The value of 'm_plen' will be wrong once we enable
> the non 4K-sized lcluster.

I'd like to make the last sentence into a separate paragraph.
I could update it when applying.

>
> Signed-off-by: Yue Hu <[email protected]>

Otherwise it looks good to me,
Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

> ---
> fs/erofs/zmap.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 572f0b8151ba..d58549ca1df9 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -141,7 +141,7 @@ struct z_erofs_maprecorder {
> u8 type, headtype;
> u16 clusterofs;
> u16 delta[2];
> - erofs_blk_t pblk, compressedlcs;
> + erofs_blk_t pblk, compressedblks;
> erofs_off_t nextpackoff;
> };
>
> @@ -192,7 +192,7 @@ static int legacy_load_cluster_from_disk(struct z_erofs_maprecorder *m,
> DBG_BUGON(1);
> return -EFSCORRUPTED;
> }
> - m->compressedlcs = m->delta[0] &
> + m->compressedblks = m->delta[0] &
> ~Z_EROFS_VLE_DI_D0_CBLKCNT;
> m->delta[0] = 1;
> }
> @@ -293,7 +293,7 @@ static int unpack_compacted_index(struct z_erofs_maprecorder *m,
> DBG_BUGON(1);
> return -EFSCORRUPTED;
> }
> - m->compressedlcs = lo & ~Z_EROFS_VLE_DI_D0_CBLKCNT;
> + m->compressedblks = lo & ~Z_EROFS_VLE_DI_D0_CBLKCNT;
> m->delta[0] = 1;
> return 0;
> } else if (i + 1 != (int)vcnt) {
> @@ -497,7 +497,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
> return 0;
> }
> lcn = m->lcn + 1;
> - if (m->compressedlcs)
> + if (m->compressedblks)
> goto out;
>
> err = z_erofs_load_cluster_from_disk(m, lcn, false);
> @@ -506,7 +506,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
>
> /*
> * If the 1st NONHEAD lcluster has already been handled initially w/o
> - * valid compressedlcs, which means at least it mustn't be CBLKCNT, or
> + * valid compressedblks, which means at least it mustn't be CBLKCNT, or
> * an internal implemenatation error is detected.
> *
> * The following code can also handle it properly anyway, but let's
> @@ -523,12 +523,12 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
> * if the 1st NONHEAD lcluster is actually PLAIN or HEAD type
> * rather than CBLKCNT, it's a 1 lcluster-sized pcluster.
> */
> - m->compressedlcs = 1;
> + m->compressedblks = 1 << (lclusterbits - LOG_BLOCK_SIZE);
> break;
> case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
> if (m->delta[0] != 1)
> goto err_bonus_cblkcnt;
> - if (m->compressedlcs)
> + if (m->compressedblks)
> break;
> fallthrough;
> default:
> @@ -539,7 +539,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
> return -EFSCORRUPTED;
> }
> out:
> - map->m_plen = (u64)m->compressedlcs << lclusterbits;
> + map->m_plen = (u64)m->compressedblks << LOG_BLOCK_SIZE;
> return 0;
> err_bonus_cblkcnt:
> erofs_err(m->inode->i_sb,
> --
> 2.17.1

2022-09-05 15:21:04

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH] erofs: avoid the potentially wrong m_plen for big pcluster

On 2022/8/12 14:01, Yue Hu wrote:
> Actually, 'compressedlcs' stores compressed block count rather than
> lcluster count. Therefore, the number of bits for shifting the count
> should be 'LOG_BLOCK_SIZE' rather than 'lclusterbits' although current
> lcluster size is 4K. The value of 'm_plen' will be wrong once we enable
> the non 4K-sized lcluster.
>
> Signed-off-by: Yue Hu <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,