[PATCH v2 0/8] staging: erofs: error handing and more tracepoints

[PATCH v2 0/8] staging: erofs: error handing and more tracepoints

[Gao Xiang]

change log v2:
  - fix a gcc warning reported by Greg for the patch
        staging: erofs: clean up z_erofs_map_blocks_iter


In order to avoid conflicts with cleanup patches, Chao and I think
it is better to send reviewed preview patches in the erofs mailing list
to the community in time.

So here is reviewed & tested patches right now, which clean up and
enhance the error handing and add some tracepoints for decompression.

Note that in this patchset, bare use of 'unsigned' and NULL comparison are
also fixed compared with the preview patches according to the previous
discussion in the staging mailing list.

Thanks,
Gao Xiang

Chen Gong (2):
  staging: erofs: add trace points for reading zipped data
  staging: erofs: replace BUG_ON with DBG_BUGON in data.c

Gao Xiang (6):
  staging: erofs: fix a missing endian conversion
  staging: erofs: clean up z_erofs_map_blocks_iter
  staging: erofs: complete error handing of z_erofs_map_blocks_iter
  staging: erofs: fix a bug when appling cache strategy
  staging: erofs: complete error handing of z_erofs_do_read_page
  staging: erofs: avoid magic constants when initializing clusterbits

 drivers/staging/erofs/data.c                       |  31 ++--
 drivers/staging/erofs/include/trace/events/erofs.h |  20 ++-
 drivers/staging/erofs/super.c                      |   5 +-
 drivers/staging/erofs/unzip_vle.c                  | 196 +++++++++++++--------
 4 files changed, 163 insertions(+), 89 deletions(-)

-- 
1.9.1

[PATCH v2 1/8] staging: erofs: fix a missing endian conversion

[Gao Xiang]

This patch fixes a missing endian conversion in
vle_get_logical_extent_head.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/unzip_vle.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 21874b7..d16e3dc 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1488,6 +1488,7 @@ static erofs_off_t vle_get_logical_extent_head(
 	struct super_block *const sb = inode->i_sb;
 	const unsigned int clusterbits = EROFS_SB(sb)->clusterbits;
 	const unsigned int clustersize = 1 << clusterbits;
+	unsigned int delta0;
 
 	if (page->index != blkaddr) {
 		kunmap_atomic(*kaddr_iter);
@@ -1502,12 +1503,13 @@ static erofs_off_t vle_get_logical_extent_head(
 	di = *kaddr_iter + vle_extent_blkoff(inode, lcn);
 	switch (vle_cluster_type(di)) {
 	case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
-		BUG_ON(!di->di_u.delta[0]);
-		BUG_ON(lcn < di->di_u.delta[0]);
+		delta0 = le16_to_cpu(di->di_u.delta[0]);
+		DBG_BUGON(!delta0);
+		DBG_BUGON(lcn < delta0);
 
 		ofs = vle_get_logical_extent_head(inode,
 			page_iter, kaddr_iter,
-			lcn - di->di_u.delta[0], pcn, flags);
+			lcn - delta0, pcn, flags);
 		break;
 	case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
 		*flags ^= EROFS_MAP_ZIPPED;
-- 
1.9.1

[PATCH v2 2/8] staging: erofs: clean up z_erofs_map_blocks_iter

[Gao Xiang]

This patch mainly introduces `vle_map_blocks_iter_ctx' to clean up
z_erofs_map_blocks_iter and vle_get_logical_extent_head.

It changes the return value of `vle_get_logical_extent_head' to int
for the later error handing. In addition, it also renames `pcn' to
`pblk' since only `pblk' exists in erofs compression ondisk format.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/unzip_vle.c | 140 +++++++++++++++++++++-----------------
 1 file changed, 78 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index d16e3dc..1e89ff6 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1410,6 +1410,13 @@ static int z_erofs_vle_normalaccess_readpages(
 	.readpages = z_erofs_vle_normalaccess_readpages,
 };
 
+/*
+ * Variable-sized Logical Extent (Fixed Physical Cluster) Compression Mode
+ * ---
+ * VLE compression mode attempts to compress a number of logical data into
+ * a physical cluster with a fixed size.
+ * VLE compression mode uses "struct z_erofs_vle_decompressed_index".
+ */
 #define __vle_cluster_advise(x, bit, bits) \
 	((le16_to_cpu(x) >> (bit)) & ((1 << (bits)) - 1))
 
@@ -1465,90 +1472,96 @@ static int z_erofs_vle_normalaccess_readpages(
 	return erofs_blkoff(iloc(sbi, vi->nid) + ofs);
 }
 
-/*
- * Variable-sized Logical Extent (Fixed Physical Cluster) Compression Mode
- * ---
- * VLE compression mode attempts to compress a number of logical data into
- * a physical cluster with a fixed size.
- * VLE compression mode uses "struct z_erofs_vle_decompressed_index".
- */
-static erofs_off_t vle_get_logical_extent_head(
-	struct inode *inode,
-	struct page **page_iter,
-	void **kaddr_iter,
-	unsigned int lcn,	/* logical cluster number */
-	erofs_blk_t *pcn,
-	unsigned int *flags)
+struct vle_map_blocks_iter_ctx {
+	struct inode *inode;
+	struct super_block *sb;
+	unsigned int clusterbits;
+
+	struct page **mpage_ret;
+	void **kaddr_ret;
+};
+
+static int
+vle_get_logical_extent_head(const struct vle_map_blocks_iter_ctx *ctx,
+			    unsigned int lcn,	/* logical cluster number */
+			    unsigned long long *ofs,
+			    erofs_blk_t *pblk,
+			    unsigned int *flags)
 {
-	/* for extent meta */
-	struct page *page = *page_iter;
-	erofs_blk_t blkaddr = vle_extent_blkaddr(inode, lcn);
+	const unsigned int clustersize = 1 << ctx->clusterbits;
+	const erofs_blk_t mblk = vle_extent_blkaddr(ctx->inode, lcn);
+	struct page *mpage = *ctx->mpage_ret;	/* extent metapage */
+
 	struct z_erofs_vle_decompressed_index *di;
-	unsigned long long ofs;
-	struct super_block *const sb = inode->i_sb;
-	const unsigned int clusterbits = EROFS_SB(sb)->clusterbits;
-	const unsigned int clustersize = 1 << clusterbits;
-	unsigned int delta0;
-
-	if (page->index != blkaddr) {
-		kunmap_atomic(*kaddr_iter);
-		unlock_page(page);
-		put_page(page);
+	unsigned int cluster_type, delta0;
 
-		page = erofs_get_meta_page_nofail(sb, blkaddr, false);
-		*page_iter = page;
-		*kaddr_iter = kmap_atomic(page);
+	if (mpage->index != mblk) {
+		kunmap_atomic(*ctx->kaddr_ret);
+		unlock_page(mpage);
+		put_page(mpage);
+
+		mpage = erofs_get_meta_page_nofail(ctx->sb, mblk, false);
+		*ctx->mpage_ret = mpage;
+		*ctx->kaddr_ret = kmap_atomic(mpage);
 	}
 
-	di = *kaddr_iter + vle_extent_blkoff(inode, lcn);
-	switch (vle_cluster_type(di)) {
+	di = *ctx->kaddr_ret + vle_extent_blkoff(ctx->inode, lcn);
+
+	cluster_type = vle_cluster_type(di);
+	switch (cluster_type) {
 	case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
 		delta0 = le16_to_cpu(di->di_u.delta[0]);
 		DBG_BUGON(!delta0);
 		DBG_BUGON(lcn < delta0);
 
-		ofs = vle_get_logical_extent_head(inode,
-			page_iter, kaddr_iter,
-			lcn - delta0, pcn, flags);
-		break;
+		return vle_get_logical_extent_head(ctx,
+			lcn - delta0, ofs, pblk, flags);
 	case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
 		*flags ^= EROFS_MAP_ZIPPED;
+		/* fallthrough */
 	case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
 		/* clustersize should be a power of two */
-		ofs = ((u64)lcn << clusterbits) +
+		*ofs = ((u64)lcn << ctx->clusterbits) +
 			(le16_to_cpu(di->di_clusterofs) & (clustersize - 1));
-		*pcn = le32_to_cpu(di->di_u.blkaddr);
+		*pblk = le32_to_cpu(di->di_u.blkaddr);
 		break;
 	default:
 		BUG_ON(1);
 	}
-	return ofs;
+	return 0;
 }
 
 int z_erofs_map_blocks_iter(struct inode *inode,
 	struct erofs_map_blocks *map,
 	struct page **mpage_ret, int flags)
 {
+	void *kaddr;
+	const struct vle_map_blocks_iter_ctx ctx = {
+		.inode = inode,
+		.sb = inode->i_sb,
+		.clusterbits = EROFS_I_SB(inode)->clusterbits,
+		.mpage_ret = mpage_ret,
+		.kaddr_ret = &kaddr
+	};
+	const unsigned int clustersize = 1 << ctx.clusterbits;
+	/* if both m_(l,p)len are 0, regularize l_lblk, l_lofs, etc... */
+	const bool initial = !map->m_llen;
+
 	/* logicial extent (start, end) offset */
 	unsigned long long ofs, end;
-	struct z_erofs_vle_decompressed_index *di;
-	erofs_blk_t e_blkaddr, pcn;
-	unsigned int lcn, logical_cluster_ofs, cluster_type;
+	unsigned int lcn;
 	u32 ofs_rem;
+
+	/* initialize `pblk' to keep gcc from printing foolish warnings */
+	erofs_blk_t mblk, pblk = 0;
 	struct page *mpage = *mpage_ret;
-	void *kaddr;
-	bool initial;
-	struct super_block *const sb = inode->i_sb;
-	const unsigned int clusterbits = EROFS_SB(sb)->clusterbits;
-	const unsigned int clustersize = 1 << clusterbits;
+	struct z_erofs_vle_decompressed_index *di;
+	unsigned int cluster_type, logical_cluster_ofs;
 	int err = 0;
 
-	/* if both m_(l,p)len are 0, regularize l_lblk, l_lofs, etc... */
-	initial = !map->m_llen;
-
 	/* when trying to read beyond EOF, leave it unmapped */
 	if (unlikely(map->m_la >= inode->i_size)) {
-		BUG_ON(!initial);
+		DBG_BUGON(!initial);
 		map->m_llen = map->m_la + 1 - inode->i_size;
 		map->m_la = inode->i_size;
 		map->m_flags = 0;
@@ -1561,16 +1574,16 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 	ofs = map->m_la + map->m_llen;
 
 	/* clustersize should be power of two */
-	lcn = ofs >> clusterbits;
+	lcn = ofs >> ctx.clusterbits;
 	ofs_rem = ofs & (clustersize - 1);
 
-	e_blkaddr = vle_extent_blkaddr(inode, lcn);
+	mblk = vle_extent_blkaddr(inode, lcn);
 
-	if (mpage == NULL || mpage->index != e_blkaddr) {
+	if (!mpage || mpage->index != mblk) {
 		if (mpage != NULL)
 			put_page(mpage);
 
-		mpage = erofs_get_meta_page_nofail(sb, e_blkaddr, false);
+		mpage = erofs_get_meta_page_nofail(ctx.sb, mblk, false);
 		*mpage_ret = mpage;
 	} else {
 		lock_page(mpage);
@@ -1580,8 +1593,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 	kaddr = kmap_atomic(mpage);
 	di = kaddr + vle_extent_blkoff(inode, lcn);
 
-	debugln("%s, lcn %u e_blkaddr %u e_blkoff %u", __func__, lcn,
-		e_blkaddr, vle_extent_blkoff(inode, lcn));
+	debugln("%s, lcn %u mblk %u e_blkoff %u", __func__, lcn,
+		mblk, vle_extent_blkoff(inode, lcn));
 
 	err = vle_decompressed_index_clusterofs(&logical_cluster_ofs,
 						clustersize, di);
@@ -1608,13 +1621,13 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 		/* fallthrough */
 	case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
 		if (ofs_rem == logical_cluster_ofs) {
-			pcn = le32_to_cpu(di->di_u.blkaddr);
+			pblk = le32_to_cpu(di->di_u.blkaddr);
 			goto exact_hitted;
 		}
 
 		if (ofs_rem > logical_cluster_ofs) {
 			ofs = (u64)lcn * clustersize | logical_cluster_ofs;
-			pcn = le32_to_cpu(di->di_u.blkaddr);
+			pblk = le32_to_cpu(di->di_u.blkaddr);
 			break;
 		}
 
@@ -1629,9 +1642,12 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 		/* fallthrough */
 	case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
 		/* get the correspoinding first chunk */
-		ofs = vle_get_logical_extent_head(inode, mpage_ret,
-			&kaddr, lcn, &pcn, &map->m_flags);
+		err = vle_get_logical_extent_head(&ctx, lcn, &ofs,
+						  &pblk, &map->m_flags);
 		mpage = *mpage_ret;
+
+		if (unlikely(err))
+			goto unmap_out;
 		break;
 	default:
 		errln("unknown cluster type %u at offset %llu of nid %llu",
@@ -1644,7 +1660,7 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 exact_hitted:
 	map->m_llen = end - ofs;
 	map->m_plen = clustersize;
-	map->m_pa = blknr_to_addr(pcn);
+	map->m_pa = blknr_to_addr(pblk);
 	map->m_flags |= EROFS_MAP_MAPPED;
 unmap_out:
 	kunmap_atomic(kaddr);
-- 
1.9.1

[PATCH v2 3/8] staging: erofs: complete error handing of z_erofs_map_blocks_iter

[Gao Xiang]

This patch completes error handing of z_erofs_map_blocks_iter
and vle_get_logical_extent_head, including no memory and
io error cases.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/unzip_vle.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 1e89ff6..94c35b8 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1500,7 +1500,11 @@ struct vle_map_blocks_iter_ctx {
 		unlock_page(mpage);
 		put_page(mpage);
 
-		mpage = erofs_get_meta_page_nofail(ctx->sb, mblk, false);
+		mpage = erofs_get_meta_page(ctx->sb, mblk, false);
+		if (IS_ERR(mpage)) {
+			*ctx->mpage_ret = NULL;
+			return PTR_ERR(mpage);
+		}
 		*ctx->mpage_ret = mpage;
 		*ctx->kaddr_ret = kmap_atomic(mpage);
 	}
@@ -1511,9 +1515,12 @@ struct vle_map_blocks_iter_ctx {
 	switch (cluster_type) {
 	case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
 		delta0 = le16_to_cpu(di->di_u.delta[0]);
-		DBG_BUGON(!delta0);
-		DBG_BUGON(lcn < delta0);
-
+		if (unlikely(!delta0 || delta0 > lcn)) {
+			errln("invalid NONHEAD dl0 %u at lcn %u of nid %llu",
+			      delta0, lcn, EROFS_V(ctx->inode)->nid);
+			DBG_BUGON(1);
+			return -EIO;
+		}
 		return vle_get_logical_extent_head(ctx,
 			lcn - delta0, ofs, pblk, flags);
 	case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
@@ -1526,7 +1533,10 @@ struct vle_map_blocks_iter_ctx {
 		*pblk = le32_to_cpu(di->di_u.blkaddr);
 		break;
 	default:
-		BUG_ON(1);
+		errln("unknown cluster type %u at lcn %u of nid %llu",
+		      cluster_type, lcn, EROFS_V(ctx->inode)->nid);
+		DBG_BUGON(1);
+		return -EIO;
 	}
 	return 0;
 }
@@ -1583,7 +1593,11 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 		if (mpage != NULL)
 			put_page(mpage);
 
-		mpage = erofs_get_meta_page_nofail(ctx.sb, mblk, false);
+		mpage = erofs_get_meta_page(ctx.sb, mblk, false);
+		if (IS_ERR(mpage)) {
+			err = PTR_ERR(mpage);
+			goto out;
+		}
 		*mpage_ret = mpage;
 	} else {
 		lock_page(mpage);
@@ -1646,8 +1660,11 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 						  &pblk, &map->m_flags);
 		mpage = *mpage_ret;
 
-		if (unlikely(err))
-			goto unmap_out;
+		if (unlikely(err)) {
+			if (mpage)
+				goto unmap_out;
+			goto out;
+		}
 		break;
 	default:
 		errln("unknown cluster type %u at offset %llu of nid %llu",
@@ -1671,7 +1688,7 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 		map->m_llen, map->m_plen, map->m_flags);
 
 	/* aggressively BUG_ON iff CONFIG_EROFS_FS_DEBUG is on */
-	DBG_BUGON(err < 0);
+	DBG_BUGON(err < 0 && err != -ENOMEM);
 	return err;
 }
 
-- 
1.9.1

[PATCH v2 4/8] staging: erofs: fix a bug when appling cache strategy

[Gao Xiang]

As described in Kconfig, the last compressed pack should be cached
for further reading for either `EROFS_FS_ZIP_CACHE_UNIPOLAR' or
`EROFS_FS_ZIP_CACHE_BIPOLAR' by design.

However, there is a bug in z_erofs_do_read_page, it will
switch `initial' to `false' at the very beginning before it decides
to cache the last compressed pack.

caching strategy should work properly after appling this patch.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/unzip_vle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 94c35b8..3296538 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -629,7 +629,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
 	/* go ahead the next map_blocks */
 	debugln("%s: [out-of-range] pos %llu", __func__, offset + cur);
 
-	if (!z_erofs_vle_work_iter_end(builder))
+	if (z_erofs_vle_work_iter_end(builder))
 		fe->initial = false;
 
 	map->m_la = offset + cur;
-- 
1.9.1

[PATCH v2 5/8] staging: erofs: complete error handing of z_erofs_do_read_page

[Gao Xiang]

This patch completes error handing code of z_erofs_do_read_page.
PG_error will be set when some read error happens, therefore
z_erofs_onlinepage_endio will unlock this page without setting
PG_uptodate.

Reviewed-by: Chao Yu <yucxhao0@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/unzip_vle.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 3296538..08c424c 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -611,7 +611,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
 
 	enum z_erofs_page_type page_type;
 	unsigned int cur, end, spiltted, index;
-	int err;
+	int err = 0;
 
 	/* register locked file pages as online pages in pack */
 	z_erofs_onlinepage_init(page);
@@ -638,12 +638,11 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
 	if (unlikely(err))
 		goto err_out;
 
-	/* deal with hole (FIXME! broken now) */
 	if (unlikely(!(map->m_flags & EROFS_MAP_MAPPED)))
 		goto hitted;
 
 	DBG_BUGON(map->m_plen != 1 << sbi->clusterbits);
-	BUG_ON(erofs_blkoff(map->m_pa));
+	DBG_BUGON(erofs_blkoff(map->m_pa));
 
 	err = z_erofs_vle_work_iter_begin(builder, sb, map, &fe->owned_head);
 	if (unlikely(err))
@@ -688,7 +687,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
 
 		err = z_erofs_vle_work_add_page(builder,
 			newpage, Z_EROFS_PAGE_TYPE_EXCLUSIVE);
-		if (!err)
+		if (likely(!err))
 			goto retry;
 	}
 
@@ -699,9 +698,10 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
 
 	/* FIXME! avoid the last relundant fixup & endio */
 	z_erofs_onlinepage_fixup(page, index, true);
-	++spiltted;
 
-	/* also update nr_pages and increase queued_pages */
+	/* bump up the number of spiltted parts of a page */
+	++spiltted;
+	/* also update nr_pages */
 	work->nr_pages = max_t(pgoff_t, work->nr_pages, index + 1);
 next_part:
 	/* can be used for verification */
@@ -711,16 +711,18 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
 	if (end > 0)
 		goto repeat;
 
+out:
 	/* FIXME! avoid the last relundant fixup & endio */
 	z_erofs_onlinepage_endio(page);
 
 	debugln("%s, finish page: %pK spiltted: %u map->m_llen %llu",
 		__func__, page, spiltted, map->m_llen);
-	return 0;
+	return err;
 
+	/* if some error occurred while processing this page */
 err_out:
-	/* TODO: the missing error handing cases */
-	return err;
+	SetPageError(page);
+	goto out;
 }
 
 static void z_erofs_vle_unzip_kickoff(void *ptr, int bios)
-- 
1.9.1

[PATCH v2 6/8] staging: erofs: avoid magic constants when initializing clusterbits

[Gao Xiang]

Currently erofs only supports clustersize == blocksize.
and clustersize == 2^n * blocksize will be supported in the future.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/super.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 2109b03..9e42153 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -116,9 +116,10 @@ static int superblock_read(struct super_block *sb)
 #endif
 	sbi->islotbits = ffs(sizeof(struct erofs_inode_v1)) - 1;
 #ifdef CONFIG_EROFS_FS_ZIP
-	sbi->clusterbits = 12;
+	/* TODO: clusterbits should be related to inode */
+	sbi->clusterbits = blkszbits;
 
-	if (1 << (sbi->clusterbits - 12) > Z_EROFS_CLUSTER_MAX_PAGES)
+	if (1 << (sbi->clusterbits - PAGE_SHIFT) > Z_EROFS_CLUSTER_MAX_PAGES)
 		errln("clusterbits %u is not supported on this kernel",
 			sbi->clusterbits);
 #endif
-- 
1.9.1

[PATCH v2 7/8] staging: erofs: add trace points for reading zipped data

[Gao Xiang]

From: Chen Gong <gongchen4@huawei.com>

This patch adds trace points for reading zipped data.

Signed-off-by: Chen Gong <gongchen4@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/include/trace/events/erofs.h | 20 ++++++++++++++++++--
 drivers/staging/erofs/unzip_vle.c                  | 11 +++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/include/trace/events/erofs.h b/drivers/staging/erofs/include/trace/events/erofs.h
index 5aead93..660c92f 100644
--- a/drivers/staging/erofs/include/trace/events/erofs.h
+++ b/drivers/staging/erofs/include/trace/events/erofs.h
@@ -162,7 +162,8 @@
 
 	TP_printk("dev = (%d,%d), nid = %llu, la %llu llen %llu flags %s",
 		  show_dev_nid(__entry),
-		  __entry->la, __entry->llen, show_map_flags(__entry->flags))
+		  __entry->la, __entry->llen,
+		  __entry->flags ? show_map_flags(__entry->flags) : "NULL")
 );
 
 DEFINE_EVENT(erofs__map_blocks_enter, erofs_map_blocks_flatmode_enter,
@@ -172,6 +173,13 @@
 	TP_ARGS(inode, map, flags)
 );
 
+DEFINE_EVENT(erofs__map_blocks_enter, z_erofs_map_blocks_iter_enter,
+	TP_PROTO(struct inode *inode, struct erofs_map_blocks *map,
+		 unsigned int flags),
+
+	TP_ARGS(inode, map, flags)
+);
+
 DECLARE_EVENT_CLASS(erofs__map_blocks_exit,
 	TP_PROTO(struct inode *inode, struct erofs_map_blocks *map,
 		 unsigned int flags, int ret),
@@ -204,7 +212,8 @@
 
 	TP_printk("dev = (%d,%d), nid = %llu, flags %s "
 		  "la %llu pa %llu llen %llu plen %llu mflags %s ret %d",
-		  show_dev_nid(__entry), show_map_flags(__entry->flags),
+		  show_dev_nid(__entry),
+		  __entry->flags ? show_map_flags(__entry->flags) : "NULL",
 		  __entry->la, __entry->pa, __entry->llen, __entry->plen,
 		  show_mflags(__entry->mflags), __entry->ret)
 );
@@ -216,6 +225,13 @@
 	TP_ARGS(inode, map, flags, ret)
 );
 
+DEFINE_EVENT(erofs__map_blocks_exit, z_erofs_map_blocks_iter_exit,
+	TP_PROTO(struct inode *inode, struct erofs_map_blocks *map,
+		 unsigned int flags, int ret),
+
+	TP_ARGS(inode, map, flags, ret)
+);
+
 TRACE_EVENT(erofs_destroy_inode,
 	TP_PROTO(struct inode *inode),
 
diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 08c424c..2b9b313 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -13,6 +13,8 @@
 #include "unzip_vle.h"
 #include <linux/prefetch.h>
 
+#include <trace/events/erofs.h>
+
 static struct workqueue_struct *z_erofs_workqueue __read_mostly;
 static struct kmem_cache *z_erofs_workgroup_cachep __read_mostly;
 
@@ -613,6 +615,8 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
 	unsigned int cur, end, spiltted, index;
 	int err = 0;
 
+	trace_erofs_readpage(page, false);
+
 	/* register locked file pages as online pages in pack */
 	z_erofs_onlinepage_init(page);
 
@@ -1348,6 +1352,9 @@ static inline int __z_erofs_vle_normalaccess_readpages(
 	struct page *head = NULL;
 	LIST_HEAD(pagepool);
 
+	trace_erofs_readpages(mapping->host, lru_to_page(pages),
+			      nr_pages, false);
+
 #if (EROFS_FS_ZIP_CACHE_LVL >= 2)
 	f.cachedzone_la = (erofs_off_t)lru_to_page(pages)->index << PAGE_SHIFT;
 #endif
@@ -1571,6 +1578,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 	unsigned int cluster_type, logical_cluster_ofs;
 	int err = 0;
 
+	trace_z_erofs_map_blocks_iter_enter(inode, map, flags);
+
 	/* when trying to read beyond EOF, leave it unmapped */
 	if (unlikely(map->m_la >= inode->i_size)) {
 		DBG_BUGON(!initial);
@@ -1689,6 +1698,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 		__func__, map->m_la, map->m_pa,
 		map->m_llen, map->m_plen, map->m_flags);
 
+	trace_z_erofs_map_blocks_iter_exit(inode, map, flags, err);
+
 	/* aggressively BUG_ON iff CONFIG_EROFS_FS_DEBUG is on */
 	DBG_BUGON(err < 0 && err != -ENOMEM);
 	return err;
-- 
1.9.1

[PATCH v2 8/8] staging: erofs: replace BUG_ON with DBG_BUGON in data.c

[Gao Xiang]

From: Chen Gong <gongchen4@huawei.com>

This patch replace BUG_ON with DBG_BUGON in data.c, and add necessary
error handler.

Signed-off-by: Chen Gong <gongchen4@huawei.com>
Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/data.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index e191610..6384f73 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -25,7 +25,7 @@ static inline void read_endio(struct bio *bio)
 		struct page *page = bvec->bv_page;
 
 		/* page is already locked */
-		BUG_ON(PageUptodate(page));
+		DBG_BUGON(PageUptodate(page));
 
 		if (unlikely(err))
 			SetPageError(page);
@@ -110,12 +110,12 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 	struct erofs_map_blocks *map,
 	int flags)
 {
+	int err = 0;
 	erofs_blk_t nblocks, lastblk;
 	u64 offset = map->m_la;
 	struct erofs_vnode *vi = EROFS_V(inode);
 
 	trace_erofs_map_blocks_flatmode_enter(inode, map, flags);
-	BUG_ON(is_inode_layout_compression(inode));
 
 	nblocks = DIV_ROUND_UP(inode->i_size, PAGE_SIZE);
 	lastblk = nblocks - is_inode_layout_inline(inode);
@@ -142,18 +142,27 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 		map->m_plen = inode->i_size - offset;
 
 		/* inline data should locate in one meta block */
-		BUG_ON(erofs_blkoff(map->m_pa) + map->m_plen > PAGE_SIZE);
+		if (erofs_blkoff(map->m_pa) + map->m_plen > PAGE_SIZE) {
+			DBG_BUGON(1);
+			err = -EIO;
+			goto err_out;
+		}
+
 		map->m_flags |= EROFS_MAP_META;
 	} else {
 		errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
 			vi->nid, inode->i_size, map->m_la);
-		BUG();
+		DBG_BUGON(1);
+		err = -EIO;
+		goto err_out;
 	}
 
 out:
 	map->m_llen = map->m_plen;
+
+err_out:
 	trace_erofs_map_blocks_flatmode_exit(inode, map, flags, 0);
-	return 0;
+	return err;
 }
 
 #ifdef CONFIG_EROFS_FS_ZIP
@@ -209,7 +218,7 @@ static inline struct bio *erofs_read_raw_page(
 	erofs_off_t current_block = (erofs_off_t)page->index;
 	int err;
 
-	BUG_ON(!nblocks);
+	DBG_BUGON(!nblocks);
 
 	if (PageUptodate(page)) {
 		err = 0;
@@ -252,7 +261,7 @@ static inline struct bio *erofs_read_raw_page(
 		}
 
 		/* for RAW access mode, m_plen must be equal to m_llen */
-		BUG_ON(map.m_plen != map.m_llen);
+		DBG_BUGON(map.m_plen != map.m_llen);
 
 		blknr = erofs_blknr(map.m_pa);
 		blkoff = erofs_blkoff(map.m_pa);
@@ -262,7 +271,7 @@ static inline struct bio *erofs_read_raw_page(
 			void *vsrc, *vto;
 			struct page *ipage;
 
-			BUG_ON(map.m_plen > PAGE_SIZE);
+			DBG_BUGON(map.m_plen > PAGE_SIZE);
 
 			ipage = erofs_get_meta_page(inode->i_sb, blknr, 0);
 
@@ -289,7 +298,7 @@ static inline struct bio *erofs_read_raw_page(
 		}
 
 		/* pa must be block-aligned for raw reading */
-		BUG_ON(erofs_blkoff(map.m_pa) != 0);
+		DBG_BUGON(erofs_blkoff(map.m_pa));
 
 		/* max # of continuous pages */
 		if (nblocks > DIV_ROUND_UP(map.m_plen, PAGE_SIZE))
@@ -357,7 +366,7 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
 	if (IS_ERR(bio))
 		return PTR_ERR(bio);
 
-	BUG_ON(bio != NULL);	/* since we have only one bio -- must be NULL */
+	DBG_BUGON(bio);	/* since we have only one bio -- must be NULL */
 	return 0;
 }
 
@@ -395,7 +404,7 @@ static int erofs_raw_access_readpages(struct file *filp,
 		/* pages could still be locked */
 		put_page(page);
 	}
-	BUG_ON(!list_empty(pages));
+	DBG_BUGON(!list_empty(pages));
 
 	/* the rare case (end in gaps) */
 	if (unlikely(bio != NULL))
-- 
1.9.1

Re: [PATCH v2 0/8] staging: erofs: error handing and more tracepoints

[Greg Kroah-Hartman]

On Tue, Sep 18, 2018 at 10:25:32PM +0800, Gao Xiang wrote:
> change log v2:
>   - fix a gcc warning reported by Greg for the patch
>         staging: erofs: clean up z_erofs_map_blocks_iter
> 
> 
> In order to avoid conflicts with cleanup patches, Chao and I think
> it is better to send reviewed preview patches in the erofs mailing list
> to the community in time.
> 
> So here is reviewed & tested patches right now, which clean up and
> enhance the error handing and add some tracepoints for decompression.
> 
> Note that in this patchset, bare use of 'unsigned' and NULL comparison are
> also fixed compared with the preview patches according to the previous
> discussion in the staging mailing list.

Thanks for the resend, looks good.

greg k-h

Re: [PATCH v2 0/8] staging: erofs: error handing and more tracepoints

[Gao Xiang]

Hi Greg,

On 2018/9/18 22:36, Greg Kroah-Hartman wrote:
> On Tue, Sep 18, 2018 at 10:25:32PM +0800, Gao Xiang wrote:
>> change log v2:
>>   - fix a gcc warning reported by Greg for the patch
>>         staging: erofs: clean up z_erofs_map_blocks_iter
>>
>>
>> In order to avoid conflicts with cleanup patches, Chao and I think
>> it is better to send reviewed preview patches in the erofs mailing list
>> to the community in time.
>>
>> So here is reviewed & tested patches right now, which clean up and
>> enhance the error handing and add some tracepoints for decompression.
>>
>> Note that in this patchset, bare use of 'unsigned' and NULL comparison are
>> also fixed compared with the preview patches according to the previous
>> discussion in the staging mailing list.
> 
> Thanks for the resend, looks good.

Thanks for applying...
p.s. sorry for just expression... I mean "with my pleasure", sorry about my english...

Thanks,
Gao Xiang

> 
> greg k-h
>