[PATCH 01/11] erofs: fix compile error

[PATCH 01/11] erofs: fix compile error

[Chao Yu]

Without CONFIG_EROFS_FS_XATTR, compiler will complain us, fix it.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/internal.h | 2 ++
 fs/erofs/xattr.c    | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 934f750975f2..0606a50a0067 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -350,7 +350,9 @@ int erofs_namei(struct inode *dir, struct qstr *name,
 	erofs_nid_t *nid, unsigned *d_type);
 
 /* xattr.c */
+#ifdef CONFIG_EROFS_FS_XATTR
 extern const struct xattr_handler *erofs_xattr_handlers[];
+#endif
 
 extern void test_meta_ops(struct super_block *sb);
 
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index f27ee416542d..fb8f3f1b3aab 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -512,6 +512,7 @@ const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
 };
 #endif
 
+#ifdef CONFIG_EROFS_FS_XATTR
 const struct xattr_handler *erofs_xattr_handlers[] = {
 	&erofs_xattr_user_handler,
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
@@ -524,6 +525,7 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
 #endif
 	NULL,
 };
+#endif
 
 struct listxattr_iter {
 	struct xattr_iter it;
-- 
2.18.0.rc1

[PATCH 02/11] erofs: fix to avoid potential overflow

[Chao Yu]

Previsouly, we use 32bit unsigned variable to store 64bits page->index's
value, it will cause potential overflow, fix it.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/data.c     | 8 ++++----
 fs/erofs/internal.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 432581d4ed0c..e76de71c85ff 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -200,12 +200,12 @@ static inline struct bio *erofs_read_raw_page(
 	struct bio *bio,
 	struct address_space *mapping,
 	struct page *page,
-	unsigned *last_block,
+	erofs_off_t *last_block,
 	unsigned nblocks,
 	bool ra)
 {
 	struct inode *inode = mapping->host;
-	unsigned current_block = page->index;
+	erofs_off_t current_block = (erofs_off_t)page->index;
 	int err;
 
 	BUG_ON(!nblocks);
@@ -334,7 +334,7 @@ static inline struct bio *erofs_read_raw_page(
  */
 static int erofs_raw_access_readpage(struct file *file, struct page *page)
 {
-	unsigned last_block;
+	erofs_off_t last_block;
 	struct bio *bio;
 
 	bio = erofs_read_raw_page(NULL, page->mapping,
@@ -351,7 +351,7 @@ static int erofs_raw_access_readpages(struct file *filp,
 	struct address_space *mapping,
 	struct list_head *pages, unsigned nr_pages)
 {
-	unsigned last_block;
+	erofs_off_t last_block;
 	struct bio *bio = NULL;
 	gfp_t gfp = readahead_gfp_mask(mapping);
 
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 0606a50a0067..7670c2ed53d2 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -127,7 +127,7 @@ static inline erofs_off_t iloc(struct erofs_sb_info *sbi, erofs_nid_t nid)
 
 #define erofs_blknr(addr)       ((addr) / EROFS_BLKSIZ)
 #define erofs_blkoff(addr)      ((addr) % EROFS_BLKSIZ)
-#define blknr_to_addr(nr)       ((nr) * EROFS_BLKSIZ)
+#define blknr_to_addr(nr)       ((erofs_off_t)(nr) * EROFS_BLKSIZ)
 
 #define inode_set_inited_xattr(inode)   (EROFS_V(inode)->flags |= 1)
 #define inode_has_inited_xattr(inode)   (EROFS_V(inode)->flags & 1)
-- 
2.18.0.rc1

[PATCH 03/11] erofs: fix missing endian conversion

[Chao Yu]

This patch fixes to add missing endian conversion.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/dir.c   | 13 ++++++++-----
 fs/erofs/namei.c |  2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index 39870e165007..5c5be7a88f71 100644
--- a/fs/erofs/dir.c
+++ b/fs/erofs/dir.c
@@ -40,17 +40,19 @@ static int erofs_fill_dentries(struct dir_context *ctx,
 		unsigned char dbg_namebuf[EROFS_NAME_LEN];
 #endif
 
+		nameoff = le16_to_cpu(de->nameoff);
+
 		if (unlikely(de->file_type < EROFS_FT_MAX))
 			d_type = erofs_filetype_table[de->file_type];
 		else
 			d_type = DT_UNKNOWN;
 
-		de_name = (char *)dentry_blk + de->nameoff;
+		de_name = (char *)dentry_blk + nameoff;
 
 		de_namelen = unlikely(de + 1 >= end) ?
 			/* last directory entry */
-			strnlen(de_name, maxsize - de->nameoff) :
-			de[1].nameoff - de->nameoff;
+			strnlen(de_name, maxsize - nameoff) :
+			le16_to_cpu(de[1].nameoff) - nameoff;
 
 		/* the corrupted directory found */
 		BUG_ON(de_namelen < 0);
@@ -64,7 +66,8 @@ static int erofs_fill_dentries(struct dir_context *ctx,
 			dbg_namebuf, de_namelen, d_type);
 #endif
 
-		if (!dir_emit(ctx, de_name, de_namelen, de->nid, d_type))
+		if (!dir_emit(ctx, de_name, de_namelen,
+					le64_to_cpu(de->nid), d_type))
 			/* stoped by some reason */
 			return 1;
 		++de;
@@ -96,7 +99,7 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
 		lock_page(dentry_page);
 		de = (struct erofs_dirent *)kmap(dentry_page);
 
-		nameoff = de->nameoff;
+		nameoff = le16_to_cpu(de->nameoff);
 
 		if (unlikely(nameoff < sizeof(struct erofs_dirent) ||
 			nameoff >= PAGE_SIZE)) {
diff --git a/fs/erofs/namei.c b/fs/erofs/namei.c
index 506974fd3a88..e30294b279d1 100644
--- a/fs/erofs/namei.c
+++ b/fs/erofs/namei.c
@@ -125,7 +125,7 @@ static struct page *find_target_block_classic(
 
 			matched = min(startprfx, endprfx);
 
-			dname.name = (u8 *)de + de->nameoff;
+			dname.name = (u8 *)de + nameoff;
 			dname.len = ndirents == 1 ?
 				/* since the rest of the last page is 0 */
 				EROFS_BLKSIZ - nameoff
-- 
2.18.0.rc1

[PATCH 04/11] erofs: fix to do endian conversion correctly

[Chao Yu]

erofs_super_block::root_nid is type of __le16, so it needs to use
le16_to_cpu for endian conversion instead of le64_to_cpu, fix it.

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

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 8da471155da2..0db207056bc5 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -114,7 +114,7 @@ static int superblock_read(struct super_block *sb)
 	sbi->clusterbits = 12;
 #endif
 
-	sbi->root_nid = le64_to_cpu(layout->root_nid);
+	sbi->root_nid = le16_to_cpu(layout->root_nid);
 	sbi->inos = le64_to_cpu(layout->inos);
 
 	sbi->build_time = le64_to_cpu(layout->build_time);
-- 
2.18.0.rc1

[PATCH 05/11] erofs: fix to handle return value of erofs_init_page_bundle() correctly

[Chao Yu]

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

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0db207056bc5..e6d4efd7c043 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -189,8 +189,8 @@ static int erofs_read_super(struct super_block *sb,
 		infoln("root inode @ nid %llu", ROOT_NID(sbi));
 #ifdef CONFIG_EROFS_FS_PAGE_BUNDLE
 	sbi->ibundle = erofs_init_page_bundle(sb);
-	if (sbi->ibundle == NULL) {
-		err = -ENOMEM;
+	if (IS_ERR(sbi->ibundle)) {
+		err = PTR_ERR(sbi->ibundle);
 		goto err_sbi;
 	}
 #endif
-- 
2.18.0.rc1

[PATCH 06/11] erofs: fix to return correct value of alloc_inode

[Chao Yu]

We should return NULL value in error path of alloc_inode(), otherwise,
VFS will treat any non-null return value as value inode, result in
invalid pointer dereference.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/super.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index e6d4efd7c043..3fcb56fd988e 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -40,12 +40,12 @@ static struct inode *alloc_inode(struct super_block *sb)
 	struct erofs_vnode *vi =
 		kmem_cache_alloc(erofs_inode_cachep, GFP_KERNEL);
 
-	if (vi != NULL) {
-		/* zero out everything except vfs_inode */
-		memset(vi, 0, offsetof(struct erofs_vnode, vfs_inode));
-		return &vi->vfs_inode;
-	}
-	return ERR_PTR(-ENOMEM);
+	if (!vi)
+		return NULL;
+
+	/* zero out everything except vfs_inode */
+	memset(vi, 0, offsetof(struct erofs_vnode, vfs_inode));
+	return &vi->vfs_inode;
 }
 
 static void i_callback(struct rcu_head *head)
-- 
2.18.0.rc1

[PATCH 07/11] erofs: remove unused EROFS_XATTR_INDEX_ADVISE

[Chao Yu]

EROFS_XATTR_INDEX_ADVISE is never used in anywhere of erofs, just
remove it.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/erofs_fs.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 96feebc0bbec..fc92213c02e5 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -150,7 +150,6 @@ struct erofs_xattr_ibody_header {
 #define EROFS_XATTR_INDEX_TRUSTED           4
 #define EROFS_XATTR_INDEX_LUSTRE            5
 #define EROFS_XATTR_INDEX_SECURITY          6
-#define EROFS_XATTR_INDEX_ADVISE            7
 
 /* xattr entry (for both inline & shared xattrs) */
 struct erofs_xattr_entry {
-- 
2.18.0.rc1

[PATCH 08/11] erofs: support special inode

[Chao Yu]

This patch adds to support special inode, such as block dev, char,
socket, pipe inode.

Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/inode.c    | 33 +++++++++++++++++++++++++++++++--
 fs/erofs/internal.h |  1 +
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 94e827b57a5b..3dbce0cf41ff 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -30,8 +30,16 @@ static int read_inode(struct inode *inode, void *data)
 		vi->inode_isize = sizeof(struct erofs_inode_v2);
 		vi->xattr_isize = ondisk_xattr_ibody_size(v2->i_xattr_icount);
 
-		vi->raw_blkaddr = le32_to_cpu(v2->i_u.raw_blkaddr);
 		inode->i_mode = le16_to_cpu(v2->i_mode);
+		if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
+						S_ISLNK(inode->i_mode)) {
+			vi->raw_blkaddr = le32_to_cpu(v2->i_u.raw_blkaddr);
+		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
+			inode->i_rdev =
+				new_decode_dev(le32_to_cpu(v2->i_u.rdev));
+		} else if (S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
+			inode->i_rdev = 0;
+		}
 
 		i_uid_write(inode, le32_to_cpu(v2->i_uid));
 		i_gid_write(inode, le32_to_cpu(v2->i_gid));
@@ -50,8 +58,16 @@ static int read_inode(struct inode *inode, void *data)
 		vi->inode_isize = sizeof(struct erofs_inode_v1);
 		vi->xattr_isize = ondisk_xattr_ibody_size(v1->i_xattr_icount);
 
-		vi->raw_blkaddr = le32_to_cpu(v1->i_u.raw_blkaddr);
 		inode->i_mode = le16_to_cpu(v1->i_mode);
+		if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
+						S_ISLNK(inode->i_mode)) {
+			vi->raw_blkaddr = le32_to_cpu(v1->i_u.raw_blkaddr);
+		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
+			inode->i_rdev =
+				new_decode_dev(le32_to_cpu(v1->i_u.rdev));
+		} else if (S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
+			inode->i_rdev = 0;
+		}
 
 		i_uid_write(inode, le16_to_cpu(v1->i_uid));
 		i_gid_write(inode, le16_to_cpu(v1->i_gid));
@@ -168,6 +184,12 @@ int fill_inode(struct inode *inode, int isdir)
 				&page_symlink_inode_operations;
 #endif
 			inode_nohighmem(inode);
+		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) ||
+			S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
+#ifdef CONFIG_EROFS_FS_XATTR
+			inode->i_op = &erofs_special_inode_operations;
+#endif
+			init_special_inode(inode, inode->i_mode, inode->i_rdev);
 		} else {
 			err = -EIO;
 			goto out_unlock;
@@ -244,6 +266,13 @@ const struct inode_operations erofs_symlink_xattr_iops = {
 	.listxattr = erofs_listxattr,
 #endif
 };
+
+const struct inode_operations erofs_special_inode_operations = {
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 9, 0))
+		.listxattr = erofs_listxattr,
+#endif
+};
+
 #endif
 
 #if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 2, 0))
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 7670c2ed53d2..43620c07044d 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -364,6 +364,7 @@ extern const struct inode_operations simple_symlink_inode_operations;
 #ifdef CONFIG_EROFS_FS_XATTR
 extern const struct inode_operations erofs_symlink_xattr_iops;
 extern const struct inode_operations erofs_fast_symlink_xattr_iops;
+extern const struct inode_operations erofs_special_inode_operations;
 #endif
 
 static inline void set_inode_fast_symlink(struct inode *inode)
-- 
2.18.0.rc1

[PATCH 08/11] erofs: support special inode

[Gao Xiang]

Hi Guifu and Chao,

On 2018/6/22 10:01, Chao Yu wrote:
> This patch adds to support special inode, such as block dev, char,
> socket, pipe inode.
> 

To Guifu,

Could you test this patch based on your latest _erofs_mkfs_ which supports special inodes?
--- either for device files, named pipe files or socket files.

If this patch works fine, could you also reply with a "Tested-by:" tag?

> Signed-off-by: Chao Yu <yuchao0 at huawei.com>
> ---
>  fs/erofs/inode.c    | 33 +++++++++++++++++++++++++++++++--
>  fs/erofs/internal.h |  1 +
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index 94e827b57a5b..3dbce0cf41ff 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -30,8 +30,16 @@ static int read_inode(struct inode *inode, void *data)
>  		vi->inode_isize = sizeof(struct erofs_inode_v2);
>  		vi->xattr_isize = ondisk_xattr_ibody_size(v2->i_xattr_icount);
>  
> -		vi->raw_blkaddr = le32_to_cpu(v2->i_u.raw_blkaddr);
>  		inode->i_mode = le16_to_cpu(v2->i_mode);
> +		if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> +						S_ISLNK(inode->i_mode)) {
> +			vi->raw_blkaddr = le32_to_cpu(v2->i_u.raw_blkaddr);
> +		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
> +			inode->i_rdev =
> +				new_decode_dev(le32_to_cpu(v2->i_u.rdev));
> +		} else if (S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
> +			inode->i_rdev = 0;
> +		}
To Chao,

It seems a default case missing here, and an error handling could be also added...

How about returning -EIO and furthermore
BUG_ON if CONFIG_EROFS_FS_DEBUG is enabled as well ?
(for commercial integrated test, it is hard to detect those error if just -EIO is returned.)

 18 config EROFS_FS_DEBUG
 19         bool "EROFS debugging feature"
 20         depends on EROFS_FS
 21         help
 22           Print erofs debugging messages and enable more BUG_ONs
 23           which check the filesystem consistency aggressively.
 24
 25           For daily use, say N.

So will the rest error handling code, I think.
How do you think?
If you also agree with that, let's add them gradually in the future.

>  
>  		i_uid_write(inode, le32_to_cpu(v2->i_uid));
>  		i_gid_write(inode, le32_to_cpu(v2->i_gid));
> @@ -50,8 +58,16 @@ static int read_inode(struct inode *inode, void *data)
>  		vi->inode_isize = sizeof(struct erofs_inode_v1);
>  		vi->xattr_isize = ondisk_xattr_ibody_size(v1->i_xattr_icount);
>  
> -		vi->raw_blkaddr = le32_to_cpu(v1->i_u.raw_blkaddr);
>  		inode->i_mode = le16_to_cpu(v1->i_mode);
> +		if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> +						S_ISLNK(inode->i_mode)) {
> +			vi->raw_blkaddr = le32_to_cpu(v1->i_u.raw_blkaddr);
> +		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
> +			inode->i_rdev =
> +				new_decode_dev(le32_to_cpu(v1->i_u.rdev));
> +		} else if (S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
> +			inode->i_rdev = 0;
> +		}

ditto,


The other code looks fine for me,
I think a basic test is needed for the function of this patch tho.

If you send out the next patch, you could add:
Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>

Thanks,
Gao Xiang

>  
>  		i_uid_write(inode, le16_to_cpu(v1->i_uid));
>  		i_gid_write(inode, le16_to_cpu(v1->i_gid));
> @@ -168,6 +184,12 @@ int fill_inode(struct inode *inode, int isdir)
>  				&page_symlink_inode_operations;
>  #endif
>  			inode_nohighmem(inode);
> +		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) ||
> +			S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
> +#ifdef CONFIG_EROFS_FS_XATTR
> +			inode->i_op = &erofs_special_inode_operations;
> +#endif
> +			init_special_inode(inode, inode->i_mode, inode->i_rdev);
>  		} else {
>  			err = -EIO;
>  			goto out_unlock;
> @@ -244,6 +266,13 @@ const struct inode_operations erofs_symlink_xattr_iops = {
>  	.listxattr = erofs_listxattr,
>  #endif
>  };
> +
> +const struct inode_operations erofs_special_inode_operations = {
> +#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 9, 0))
> +		.listxattr = erofs_listxattr,
> +#endif
> +};
> +
>  #endif
>  
>  #if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 2, 0))
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 7670c2ed53d2..43620c07044d 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -364,6 +364,7 @@ extern const struct inode_operations simple_symlink_inode_operations;
>  #ifdef CONFIG_EROFS_FS_XATTR
>  extern const struct inode_operations erofs_symlink_xattr_iops;
>  extern const struct inode_operations erofs_fast_symlink_xattr_iops;
> +extern const struct inode_operations erofs_special_inode_operations;
>  #endif
>  
>  static inline void set_inode_fast_symlink(struct inode *inode)
> 

[PATCH 08/11] erofs: support special inode

[Chao Yu]

Hi Xiang,

On 2018/7/3 13:31, Gao Xiang wrote:
> Hi Guifu and Chao,
> 
> On 2018/6/22 10:01, Chao Yu wrote:
>> This patch adds to support special inode, such as block dev, char,
>> socket, pipe inode.
>>
> 
> To Guifu,
> 
> Could you test this patch based on your latest _erofs_mkfs_ which supports special inodes?
> --- either for device files, named pipe files or socket files.
> 
> If this patch works fine, could you also reply with a "Tested-by:" tag?
> 
>> Signed-off-by: Chao Yu <yuchao0 at huawei.com>
>> ---
>>  fs/erofs/inode.c    | 33 +++++++++++++++++++++++++++++++--
>>  fs/erofs/internal.h |  1 +
>>  2 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
>> index 94e827b57a5b..3dbce0cf41ff 100644
>> --- a/fs/erofs/inode.c
>> +++ b/fs/erofs/inode.c
>> @@ -30,8 +30,16 @@ static int read_inode(struct inode *inode, void *data)
>>  		vi->inode_isize = sizeof(struct erofs_inode_v2);
>>  		vi->xattr_isize = ondisk_xattr_ibody_size(v2->i_xattr_icount);
>>  
>> -		vi->raw_blkaddr = le32_to_cpu(v2->i_u.raw_blkaddr);
>>  		inode->i_mode = le16_to_cpu(v2->i_mode);
>> +		if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
>> +						S_ISLNK(inode->i_mode)) {
>> +			vi->raw_blkaddr = le32_to_cpu(v2->i_u.raw_blkaddr);
>> +		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
>> +			inode->i_rdev =
>> +				new_decode_dev(le32_to_cpu(v2->i_u.rdev));
>> +		} else if (S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
>> +			inode->i_rdev = 0;
>> +		}
> To Chao,
> 
> It seems a default case missing here, and an error handling could be also added...

Yes, that's right, let me add it.

> 
> How about returning -EIO and furthermore
> BUG_ON if CONFIG_EROFS_FS_DEBUG is enabled as well ?
> (for commercial integrated test, it is hard to detect those error if just -EIO is returned.)
> 
>  18 config EROFS_FS_DEBUG
>  19         bool "EROFS debugging feature"
>  20         depends on EROFS_FS
>  21         help
>  22           Print erofs debugging messages and enable more BUG_ONs
>  23           which check the filesystem consistency aggressively.
>  24
>  25           For daily use, say N.
> 
> So will the rest error handling code, I think.
> How do you think?
> If you also agree with that, let's add them gradually in the future.

Yes, agree, that will be kind to erofs developers or testers to find bugs,
please go ahead. :)

> 
>>  
>>  		i_uid_write(inode, le32_to_cpu(v2->i_uid));
>>  		i_gid_write(inode, le32_to_cpu(v2->i_gid));
>> @@ -50,8 +58,16 @@ static int read_inode(struct inode *inode, void *data)
>>  		vi->inode_isize = sizeof(struct erofs_inode_v1);
>>  		vi->xattr_isize = ondisk_xattr_ibody_size(v1->i_xattr_icount);
>>  
>> -		vi->raw_blkaddr = le32_to_cpu(v1->i_u.raw_blkaddr);
>>  		inode->i_mode = le16_to_cpu(v1->i_mode);
>> +		if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
>> +						S_ISLNK(inode->i_mode)) {
>> +			vi->raw_blkaddr = le32_to_cpu(v1->i_u.raw_blkaddr);
>> +		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
>> +			inode->i_rdev =
>> +				new_decode_dev(le32_to_cpu(v1->i_u.rdev));
>> +		} else if (S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
>> +			inode->i_rdev = 0;
>> +		}
> 
> ditto,

Right.

> 
> 
> The other code looks fine for me,
> I think a basic test is needed for the function of this patch tho.

Yeah, I hope Guifu will leave the time for testing the patch.

> 
> If you send out the next patch, you could add:
> Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>

Thanks for the review.

Thanks,

> 
> Thanks,
> Gao Xiang
> 
>>  
>>  		i_uid_write(inode, le16_to_cpu(v1->i_uid));
>>  		i_gid_write(inode, le16_to_cpu(v1->i_gid));
>> @@ -168,6 +184,12 @@ int fill_inode(struct inode *inode, int isdir)
>>  				&page_symlink_inode_operations;
>>  #endif
>>  			inode_nohighmem(inode);
>> +		} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) ||
>> +			S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
>> +#ifdef CONFIG_EROFS_FS_XATTR
>> +			inode->i_op = &erofs_special_inode_operations;
>> +#endif
>> +			init_special_inode(inode, inode->i_mode, inode->i_rdev);
>>  		} else {
>>  			err = -EIO;
>>  			goto out_unlock;
>> @@ -244,6 +266,13 @@ const struct inode_operations erofs_symlink_xattr_iops = {
>>  	.listxattr = erofs_listxattr,
>>  #endif
>>  };
>> +
>> +const struct inode_operations erofs_special_inode_operations = {
>> +#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 9, 0))
>> +		.listxattr = erofs_listxattr,
>> +#endif
>> +};
>> +
>>  #endif
>>  
>>  #if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 2, 0))
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index 7670c2ed53d2..43620c07044d 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -364,6 +364,7 @@ extern const struct inode_operations simple_symlink_inode_operations;
>>  #ifdef CONFIG_EROFS_FS_XATTR
>>  extern const struct inode_operations erofs_symlink_xattr_iops;
>>  extern const struct inode_operations erofs_fast_symlink_xattr_iops;
>> +extern const struct inode_operations erofs_special_inode_operations;
>>  #endif
>>  
>>  static inline void set_inode_fast_symlink(struct inode *inode)
>>

[PATCH 08/11] erofs: support special inode

[Gao Xiang]

Hi Guifu,

On 2018/7/3 21:48, Chao Yu wrote:
>> The other code looks fine for me,
>> I think a basic test is needed for the function of this patch tho.
> 
> Yeah, I hope Guifu will leave the time for testing the patch.

Could you please reply this email with some test results of this patch?

Thanks,
Gao Xiang

答复: [PATCH 08/11] erofs: support special inode

[liguifu (C)]

Hi 
# Test special device of erofs
we have test four special device, like char device, block device, pipe device and sock decive
using new_encode_dev to create device number in the image created by mkfs.erofs
specail devices node were created in the  Ubuntu 16.04 LTS 4.4.0-104-generic, 
the source codes is based on LDD3 source which coming from https://github.com/duxing2007/ldd3-examples-3.x/tree/origin/linux-4.4.y

Test char devices, block device, pipe device file
step 1, build ldd3 codes 
step 2, install scull.ko sbull.ko and make device node with mknod cmd 

$ cat  /proc/devices|grep scull
246 scull
246 scullp
246 sculla

$ cat  /proc/devices|grep sbull
251 sbull

create device file:
mknod scull0 c 246 0
mknod pipe0  c 246 4
mknod sbulla b 251 0

step 3, create sock device with custom tcp server and client test program which invoke socket.
        it will create a socket file.

step 4, create erofs image using mkfs.erofs
step 5, mount erofs image and test the device file

1) scull 
echo test > scull0, you will get it by input 
cat scull0
test

2) sbull is the same to scull

3) pipe0

(1) input cat pipe0 at a shell
(2) input echo test > pip0 at another shell
So you will recive test string displayed at the fisrt shell terminal screen

4) socket
because erofs is a read only file system , socket file will not be created at fisrt running 
socket file need unlink fisrtly and recreate by program 
otherwise we got lots of string output at terminal when running socket server program.

Tks
Li GuiFu

-----????-----
???: Gaoxiang (OS) 
????: 2018?8?1? 16:18
???: liguifu (C) <bluce.liguifu at huawei.com>
??: Yuchao (T) <yuchao0 at huawei.com>; linux-erofs at lists.ozlabs.org; Wangzhigang (Brooke) <brooke.wangzhigang at hisilicon.com>
??: Re: [PATCH 08/11] erofs: support special inode

Hi Guifu,

On 2018/7/3 21:48, Chao Yu wrote:
>> The other code looks fine for me,
>> I think a basic test is needed for the function of this patch tho.
> 
> Yeah, I hope Guifu will leave the time for testing the patch.

Could you please reply this email with some test results of this patch?

Thanks,
Gao Xiang

[PATCH 09/11] erofs: introduce parse_options()

[Chao Yu]

Introduce parse_options() for mount option parsing, now, it only
supports two options: 'user_xattr' and 'acl'.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/super.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3fcb56fd988e..390a5217bbcc 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/statfs.h>
 #include <linux/seq_file.h>
+#include <linux/parser.h>
 #include "internal.h"
 
 static struct kmem_cache *erofs_inode_cachep __read_mostly;
@@ -145,6 +146,61 @@ static void default_options(struct erofs_sb_info *sbi)
 #endif
 }
 
+enum {
+	Opt_user_xattr,
+	Opt_acl
+};
+
+static match_table_t erofs_tokens = {
+	{Opt_user_xattr, "user_xattr"},
+	{Opt_acl, "acl"},
+};
+
+static int parse_options(struct super_block *sb, char *options)
+{
+	substring_t args[MAX_OPT_ARGS];
+	char *p;
+
+	if (!options)
+		return 0;
+
+	while ((p = strsep(&options, ",")) != NULL) {
+		int token;
+
+		if (!*p)
+			continue;
+
+		args[0].to = args[0].from = NULL;
+		token = match_token(p, erofs_tokens, args);
+
+		switch (token) {
+#ifdef CONFIG_EROFS_FS_XATTR
+		case Opt_user_xattr:
+			set_opt(EROFS_SB(sb), XATTR_USER);
+			break;
+#else
+		case Opt_user_xattr:
+			infoln("user_xattr options not supported");
+			break;
+#endif
+#ifdef CONFIG_EROFS_FS_POSIX_ACL
+		case Opt_acl:
+			set_opt(EROFS_SB(sb), POSIX_ACL);
+			break;
+#else
+		case Opt_acl:
+			infoln("acl options not supported");
+			break;
+#endif
+		default:
+			infoln("Unrecognized mount option \"%s\" "
+					"or missing value", p);
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
 static int erofs_read_super(struct super_block *sb,
 	const char *dev_name, void *data, int silent)
 {
@@ -185,6 +241,10 @@ static int erofs_read_super(struct super_block *sb,
 	/* set erofs default mount options */
 	default_options(sbi);
 
+	err = parse_options(sb, data);
+	if (err)
+		goto err_sbi;
+
 	if (!silent)
 		infoln("root inode @ nid %llu", ROOT_NID(sbi));
 #ifdef CONFIG_EROFS_FS_PAGE_BUNDLE
-- 
2.18.0.rc1

[PATCH 10/11] erofs: introduce error injection infrastructure

[Chao Yu]

This patch introduces error injection infrastructure, with it, we can
inject error in any kernel exported common functions which erofs used,
so that it can force erofs running into error paths, it turns out that
tests can cover real rare paths more easily to find bugs.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/Kconfig    |  6 +++++
 fs/erofs/inode.c    |  3 ++-
 fs/erofs/internal.h | 56 +++++++++++++++++++++++++++++++++++++++++++++
 fs/erofs/super.c    | 40 +++++++++++++++++++++++++++++++-
 4 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 752f0e08d0b8..e9bab0f5600c 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -86,3 +86,9 @@ config EROFS_FS_ZIP
 
 	  If you don't want to use compression, say N.
 
+config EROFS_FAULT_INJECTION
+	bool "EROFS fault injection facility"
+	depends on EROFS_FS
+	help
+	  Test EROFS to inject faults such as ENOMEM, EIO, and so on.
+	  If unsure, say N.
diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 3dbce0cf41ff..78e94ddb3080 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -100,6 +100,7 @@ static int read_inode(struct inode *inode, void *data)
 int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
 {
 	struct erofs_vnode *vi = EROFS_V(inode);
+	struct erofs_sb_info *sbi = EROFS_I_SB(inode);
 	int mode = vi->data_mapping_mode;
 
 	BUG_ON(mode >= EROFS_INODE_LAYOUT_MAX);
@@ -110,7 +111,7 @@ int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
 
 	/* fast symlink (following ext4) */
 	if (S_ISLNK(inode->i_mode) && inode->i_size < PAGE_SIZE) {
-		char *lnk = kmalloc(inode->i_size + 1, GFP_KERNEL);
+		char *lnk = erofs_kmalloc(sbi, inode->i_size + 1, GFP_KERNEL);
 
 		if (unlikely(lnk == NULL))
 			return -ENOMEM;
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 43620c07044d..eb482b54d4b2 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -45,6 +45,22 @@
 #define DBG_BUGON(...)  ((void)0)
 #endif
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+enum {
+	FAULT_KMALLOC,
+	FAULT_MAX,
+};
+
+extern char *fault_name[FAULT_MAX];
+#define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
+
+struct erofs_fault_info {
+	atomic_t inject_ops;
+	unsigned int inject_rate;
+	unsigned int inject_type;
+};
+#endif
+
 /* EROFS_SUPER_MAGIC_V1 to represent the whole file system */
 #define EROFS_SUPER_MAGIC   EROFS_SUPER_MAGIC_V1
 
@@ -81,14 +97,54 @@ struct erofs_sb_info {
 	char *dev_name;
 
 	unsigned int mount_opt;
+
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	struct erofs_fault_info fault_info;	/* For fault injection */
+#endif
 };
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+#define erofs_show_injection_info(type)					\
+	infoln("inject %s in %s of %pS", fault_name[type], __func__,	\
+					__builtin_return_address(0))
+static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
+{
+	struct erofs_fault_info *ffi = &sbi->fault_info;
+
+	if (!ffi->inject_rate)
+		return false;
+
+	if (!IS_FAULT_SET(ffi, type))
+		return false;
+
+	atomic_inc(&ffi->inject_ops);
+	if (atomic_read(&ffi->inject_ops) >= ffi->inject_rate) {
+		atomic_set(&ffi->inject_ops, 0);
+		return true;
+	}
+	return false;
+}
+#endif
+
+static inline void *erofs_kmalloc(struct erofs_sb_info *sbi,
+					size_t size, gfp_t flags)
+{
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	if (time_to_inject(sbi, FAULT_KMALLOC)) {
+		erofs_show_injection_info(FAULT_KMALLOC);
+		return NULL;
+	}
+#endif
+	return kmalloc(size, flags);
+}
+
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
 #define EROFS_I_SB(inode) ((struct erofs_sb_info *)(inode)->i_sb->s_fs_info)
 
 /* Mount flags set via mount options or defaults */
 #define EROFS_MOUNT_XATTR_USER		0x00000010
 #define EROFS_MOUNT_POSIX_ACL		0x00000020
+#define EROFS_MOUNT_FAULT_INJECTION	0x00000040
 
 #define clear_opt(sbi, option)	((sbi)->mount_opt &= ~EROFS_MOUNT_##option)
 #define set_opt(sbi, option)	((sbi)->mount_opt |= EROFS_MOUNT_##option)
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 390a5217bbcc..b5af35ec2797 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -135,6 +135,26 @@ static int superblock_read(struct super_block *sb)
 	return ret;
 }
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+char *fault_name[FAULT_MAX] = {
+	[FAULT_KMALLOC]		= "kmalloc",
+};
+
+static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
+						unsigned int rate)
+{
+	struct erofs_fault_info *ffi = &sbi->fault_info;
+
+	if (rate) {
+		atomic_set(&ffi->inject_ops, 0);
+		ffi->inject_rate = rate;
+		ffi->inject_type = (1 << FAULT_MAX) - 1;
+	} else {
+		memset(ffi, 0, sizeof(struct erofs_fault_info));
+	}
+}
+#endif
+
 static void default_options(struct erofs_sb_info *sbi)
 {
 #ifdef CONFIG_EROFS_FS_XATTR
@@ -148,18 +168,21 @@ static void default_options(struct erofs_sb_info *sbi)
 
 enum {
 	Opt_user_xattr,
-	Opt_acl
+	Opt_acl,
+	Opt_fault_injection
 };
 
 static match_table_t erofs_tokens = {
 	{Opt_user_xattr, "user_xattr"},
 	{Opt_acl, "acl"},
+	{Opt_fault_injection, "fault_injection=%u"},
 };
 
 static int parse_options(struct super_block *sb, char *options)
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *p;
+	int arg = 0;
 
 	if (!options)
 		return 0;
@@ -192,6 +215,16 @@ static int parse_options(struct super_block *sb, char *options)
 			infoln("acl options not supported");
 			break;
 #endif
+		case Opt_fault_injection:
+			if (args->from && match_int(args, &arg))
+				return -EINVAL;
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+			erofs_build_fault_attr(EROFS_SB(sb), arg);
+			set_opt(EROFS_SB(sb), FAULT_INJECTION);
+#else
+			infoln("FAULT_INJECTION was not selected");
+#endif
+			break;
 		default:
 			infoln("Unrecognized mount option \"%s\" "
 					"or missing value", p);
@@ -451,6 +484,11 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 		seq_puts(seq, ",acl");
 	else
 		seq_puts(seq, ",noacl");
+#endif
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	if (test_opt(sbi, FAULT_INJECTION))
+		seq_printf(seq, ",fault_injection=%u",
+				sbi->fault_info.inject_rate);
 #endif
 	return 0;
 }
-- 
2.18.0.rc1

[PATCH v4] erofs: introduce error injection infrastructure

[Gao Xiang]

From: Chao Yu <yuchao0@huawei.com>

This patch introduces error injection infrastructure, with it, we can
inject error in any kernel exported common functions which erofs used,
so that it can force erofs running into error paths, it turns out that
tests can cover real rare paths more easily to find bugs.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
v4 Gao Xiang <gaoxiang25 at huawei.com>:
- avoid the global variable 'fault_name' to pollute symbol space.
  Reported-by: kbuild test robot <lkp at intel.com>
v3:
- code rebase.
v2:
- add missing Kconfig entry.
- fix double prefix 'erofs:' in erofs_show_injection_info macro.
 
 fs/erofs/Kconfig    |  6 ++++++
 fs/erofs/inode.c    |  3 ++-
 fs/erofs/internal.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/erofs/super.c    | 40 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 752f0e0..e9bab0f 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -86,3 +86,9 @@ config EROFS_FS_ZIP
 
 	  If you don't want to use compression, say N.
 
+config EROFS_FAULT_INJECTION
+	bool "EROFS fault injection facility"
+	depends on EROFS_FS
+	help
+	  Test EROFS to inject faults such as ENOMEM, EIO, and so on.
+	  If unsure, say N.
diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 3dbce0c..78e94dd 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -100,6 +100,7 @@ static int read_inode(struct inode *inode, void *data)
 int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
 {
 	struct erofs_vnode *vi = EROFS_V(inode);
+	struct erofs_sb_info *sbi = EROFS_I_SB(inode);
 	int mode = vi->data_mapping_mode;
 
 	BUG_ON(mode >= EROFS_INODE_LAYOUT_MAX);
@@ -110,7 +111,7 @@ int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
 
 	/* fast symlink (following ext4) */
 	if (S_ISLNK(inode->i_mode) && inode->i_size < PAGE_SIZE) {
-		char *lnk = kmalloc(inode->i_size + 1, GFP_KERNEL);
+		char *lnk = erofs_kmalloc(sbi, inode->i_size + 1, GFP_KERNEL);
 
 		if (unlikely(lnk == NULL))
 			return -ENOMEM;
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 43620c0..9f6f4db 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -45,6 +45,22 @@
 #define DBG_BUGON(...)  ((void)0)
 #endif
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+enum {
+	FAULT_KMALLOC,
+	FAULT_MAX,
+};
+
+extern char *erofs_fault_name[FAULT_MAX];
+#define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
+
+struct erofs_fault_info {
+	atomic_t inject_ops;
+	unsigned int inject_rate;
+	unsigned int inject_type;
+};
+#endif
+
 /* EROFS_SUPER_MAGIC_V1 to represent the whole file system */
 #define EROFS_SUPER_MAGIC   EROFS_SUPER_MAGIC_V1
 
@@ -81,14 +97,55 @@ struct erofs_sb_info {
 	char *dev_name;
 
 	unsigned int mount_opt;
+
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	struct erofs_fault_info fault_info;	/* For fault injection */
+#endif
 };
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+#define erofs_show_injection_info(type)					\
+	infoln("inject %s in %s of %pS", erofs_fault_name[type],        \
+		__func__, __builtin_return_address(0))
+
+static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
+{
+	struct erofs_fault_info *ffi = &sbi->fault_info;
+
+	if (!ffi->inject_rate)
+		return false;
+
+	if (!IS_FAULT_SET(ffi, type))
+		return false;
+
+	atomic_inc(&ffi->inject_ops);
+	if (atomic_read(&ffi->inject_ops) >= ffi->inject_rate) {
+		atomic_set(&ffi->inject_ops, 0);
+		return true;
+	}
+	return false;
+}
+#endif
+
+static inline void *erofs_kmalloc(struct erofs_sb_info *sbi,
+					size_t size, gfp_t flags)
+{
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	if (time_to_inject(sbi, FAULT_KMALLOC)) {
+		erofs_show_injection_info(FAULT_KMALLOC);
+		return NULL;
+	}
+#endif
+	return kmalloc(size, flags);
+}
+
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
 #define EROFS_I_SB(inode) ((struct erofs_sb_info *)(inode)->i_sb->s_fs_info)
 
 /* Mount flags set via mount options or defaults */
 #define EROFS_MOUNT_XATTR_USER		0x00000010
 #define EROFS_MOUNT_POSIX_ACL		0x00000020
+#define EROFS_MOUNT_FAULT_INJECTION	0x00000040
 
 #define clear_opt(sbi, option)	((sbi)->mount_opt &= ~EROFS_MOUNT_##option)
 #define set_opt(sbi, option)	((sbi)->mount_opt |= EROFS_MOUNT_##option)
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 390a521..5ac674d 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -135,6 +135,26 @@ static int superblock_read(struct super_block *sb)
 	return ret;
 }
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+char *erofs_fault_name[FAULT_MAX] = {
+	[FAULT_KMALLOC]		= "kmalloc",
+};
+
+static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
+						unsigned int rate)
+{
+	struct erofs_fault_info *ffi = &sbi->fault_info;
+
+	if (rate) {
+		atomic_set(&ffi->inject_ops, 0);
+		ffi->inject_rate = rate;
+		ffi->inject_type = (1 << FAULT_MAX) - 1;
+	} else {
+		memset(ffi, 0, sizeof(struct erofs_fault_info));
+	}
+}
+#endif
+
 static void default_options(struct erofs_sb_info *sbi)
 {
 #ifdef CONFIG_EROFS_FS_XATTR
@@ -148,18 +168,21 @@ static void default_options(struct erofs_sb_info *sbi)
 
 enum {
 	Opt_user_xattr,
-	Opt_acl
+	Opt_acl,
+	Opt_fault_injection
 };
 
 static match_table_t erofs_tokens = {
 	{Opt_user_xattr, "user_xattr"},
 	{Opt_acl, "acl"},
+	{Opt_fault_injection, "fault_injection=%u"},
 };
 
 static int parse_options(struct super_block *sb, char *options)
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *p;
+	int arg = 0;
 
 	if (!options)
 		return 0;
@@ -192,6 +215,16 @@ static int parse_options(struct super_block *sb, char *options)
 			infoln("acl options not supported");
 			break;
 #endif
+		case Opt_fault_injection:
+			if (args->from && match_int(args, &arg))
+				return -EINVAL;
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+			erofs_build_fault_attr(EROFS_SB(sb), arg);
+			set_opt(EROFS_SB(sb), FAULT_INJECTION);
+#else
+			infoln("FAULT_INJECTION was not selected");
+#endif
+			break;
 		default:
 			infoln("Unrecognized mount option \"%s\" "
 					"or missing value", p);
@@ -452,6 +485,11 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 	else
 		seq_puts(seq, ",noacl");
 #endif
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	if (test_opt(sbi, FAULT_INJECTION))
+		seq_printf(seq, ",fault_injection=%u",
+				sbi->fault_info.inject_rate);
+#endif
 	return 0;
 }
 
-- 
1.9.1

[PATCH v5] erofs: introduce error injection infrastructure

[Gao Xiang]

From: Chao Yu <yuchao0@huawei.com>

This patch introduces error injection infrastructure, with it, we can
inject error in any kernel exported common functions which erofs used,
so that it can force erofs running into error paths, it turns out that
tests can cover real rare paths more easily to find bugs.

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
v5 Gao Xiang <gaoxiang25 at huawei.com>:
- fix implicit declaration of "kmalloc" by moving
  <linux/slab.h> to internal.h
  Reported-by: kbuild test robot <lkp at intel.com>
  https://lists.01.org/pipermail/kbuild-all/2018-June/049644.html

v4 Gao Xiang <gaoxiang25 at huawei.com>:
- avoid the global variable 'fault_name' to pollute symbol space.
  Reported-by: kbuild test robot <lkp at intel.com>
  https://lists.01.org/pipermail/kbuild-all/2018-June/049660.html
v3:
- code rebase.
v2:
- add missing Kconfig entry.
- fix double prefix 'erofs:' in erofs_show_injection_info macro.

 fs/erofs/Kconfig    |  6 +++++
 fs/erofs/inode.c    |  4 ++--
 fs/erofs/internal.h | 58 +++++++++++++++++++++++++++++++++++++++++++++
 fs/erofs/super.c    | 41 ++++++++++++++++++++++++++++++--
 fs/erofs/xattr.c    |  1 -
 5 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 752f0e08d0b8..e9bab0f5600c 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -86,3 +86,9 @@ config EROFS_FS_ZIP
 
 	  If you don't want to use compression, say N.
 
+config EROFS_FAULT_INJECTION
+	bool "EROFS fault injection facility"
+	depends on EROFS_FS
+	help
+	  Test EROFS to inject faults such as ENOMEM, EIO, and so on.
+	  If unsure, say N.
diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 3dbce0cf41ff..844b999649f0 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -11,7 +11,6 @@
  * distribution for more details.
  */
 #include "internal.h"
-#include <linux/slab.h>
 #include "xattr.h"
 
 /* no locking */
@@ -100,6 +99,7 @@ static int read_inode(struct inode *inode, void *data)
 int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
 {
 	struct erofs_vnode *vi = EROFS_V(inode);
+	struct erofs_sb_info *sbi = EROFS_I_SB(inode);
 	int mode = vi->data_mapping_mode;
 
 	BUG_ON(mode >= EROFS_INODE_LAYOUT_MAX);
@@ -110,7 +110,7 @@ int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
 
 	/* fast symlink (following ext4) */
 	if (S_ISLNK(inode->i_mode) && inode->i_size < PAGE_SIZE) {
-		char *lnk = kmalloc(inode->i_size + 1, GFP_KERNEL);
+		char *lnk = erofs_kmalloc(sbi, inode->i_size + 1, GFP_KERNEL);
 
 		if (unlikely(lnk == NULL))
 			return -ENOMEM;
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 43620c07044d..2567fb83b5f6 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -23,6 +23,7 @@
 #ifdef CONFIG_EROFS_FS_PAGE_BUNDLE
 #include <linux/swap.h>
 #endif
+#include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include "erofs_fs.h"
 
@@ -45,6 +46,22 @@
 #define DBG_BUGON(...)  ((void)0)
 #endif
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+enum {
+	FAULT_KMALLOC,
+	FAULT_MAX,
+};
+
+extern char *erofs_fault_name[FAULT_MAX];
+#define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
+
+struct erofs_fault_info {
+	atomic_t inject_ops;
+	unsigned int inject_rate;
+	unsigned int inject_type;
+};
+#endif
+
 /* EROFS_SUPER_MAGIC_V1 to represent the whole file system */
 #define EROFS_SUPER_MAGIC   EROFS_SUPER_MAGIC_V1
 
@@ -81,14 +98,55 @@ struct erofs_sb_info {
 	char *dev_name;
 
 	unsigned int mount_opt;
+
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	struct erofs_fault_info fault_info;	/* For fault injection */
+#endif
 };
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+#define erofs_show_injection_info(type)					\
+	infoln("inject %s in %s of %pS", erofs_fault_name[type],        \
+		__func__, __builtin_return_address(0))
+
+static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
+{
+	struct erofs_fault_info *ffi = &sbi->fault_info;
+
+	if (!ffi->inject_rate)
+		return false;
+
+	if (!IS_FAULT_SET(ffi, type))
+		return false;
+
+	atomic_inc(&ffi->inject_ops);
+	if (atomic_read(&ffi->inject_ops) >= ffi->inject_rate) {
+		atomic_set(&ffi->inject_ops, 0);
+		return true;
+	}
+	return false;
+}
+#endif
+
+static inline void *erofs_kmalloc(struct erofs_sb_info *sbi,
+					size_t size, gfp_t flags)
+{
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	if (time_to_inject(sbi, FAULT_KMALLOC)) {
+		erofs_show_injection_info(FAULT_KMALLOC);
+		return NULL;
+	}
+#endif
+	return kmalloc(size, flags);
+}
+
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
 #define EROFS_I_SB(inode) ((struct erofs_sb_info *)(inode)->i_sb->s_fs_info)
 
 /* Mount flags set via mount options or defaults */
 #define EROFS_MOUNT_XATTR_USER		0x00000010
 #define EROFS_MOUNT_POSIX_ACL		0x00000020
+#define EROFS_MOUNT_FAULT_INJECTION	0x00000040
 
 #define clear_opt(sbi, option)	((sbi)->mount_opt &= ~EROFS_MOUNT_##option)
 #define set_opt(sbi, option)	((sbi)->mount_opt |= EROFS_MOUNT_##option)
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 390a5217bbcc..d6046d4d787b 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -12,7 +12,6 @@
  */
 #include <linux/module.h>
 #include <linux/buffer_head.h>
-#include <linux/slab.h>
 #include <linux/statfs.h>
 #include <linux/seq_file.h>
 #include <linux/parser.h>
@@ -135,6 +134,26 @@ static int superblock_read(struct super_block *sb)
 	return ret;
 }
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+char *erofs_fault_name[FAULT_MAX] = {
+	[FAULT_KMALLOC]		= "kmalloc",
+};
+
+static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
+						unsigned int rate)
+{
+	struct erofs_fault_info *ffi = &sbi->fault_info;
+
+	if (rate) {
+		atomic_set(&ffi->inject_ops, 0);
+		ffi->inject_rate = rate;
+		ffi->inject_type = (1 << FAULT_MAX) - 1;
+	} else {
+		memset(ffi, 0, sizeof(struct erofs_fault_info));
+	}
+}
+#endif
+
 static void default_options(struct erofs_sb_info *sbi)
 {
 #ifdef CONFIG_EROFS_FS_XATTR
@@ -148,18 +167,21 @@ static void default_options(struct erofs_sb_info *sbi)
 
 enum {
 	Opt_user_xattr,
-	Opt_acl
+	Opt_acl,
+	Opt_fault_injection
 };
 
 static match_table_t erofs_tokens = {
 	{Opt_user_xattr, "user_xattr"},
 	{Opt_acl, "acl"},
+	{Opt_fault_injection, "fault_injection=%u"},
 };
 
 static int parse_options(struct super_block *sb, char *options)
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *p;
+	int arg = 0;
 
 	if (!options)
 		return 0;
@@ -192,6 +214,16 @@ static int parse_options(struct super_block *sb, char *options)
 			infoln("acl options not supported");
 			break;
 #endif
+		case Opt_fault_injection:
+			if (args->from && match_int(args, &arg))
+				return -EINVAL;
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+			erofs_build_fault_attr(EROFS_SB(sb), arg);
+			set_opt(EROFS_SB(sb), FAULT_INJECTION);
+#else
+			infoln("FAULT_INJECTION was not selected");
+#endif
+			break;
 		default:
 			infoln("Unrecognized mount option \"%s\" "
 					"or missing value", p);
@@ -451,6 +483,11 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 		seq_puts(seq, ",acl");
 	else
 		seq_puts(seq, ",noacl");
+#endif
+#ifdef CONFIG_EROFS_FAULT_INJECTION
+	if (test_opt(sbi, FAULT_INJECTION))
+		seq_printf(seq, ",fault_injection=%u",
+				sbi->fault_info.inject_rate);
 #endif
 	return 0;
 }
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index fb8f3f1b3aab..aa0231768643 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -10,7 +10,6 @@
  * License.  See the file COPYING in the main directory of the Linux
  * distribution for more details.
  */
-#include <linux/slab.h>
 #include <linux/security.h>
 #include "xattr.h"
 #include <linux/version.h>
-- 
2.17.1

[PATCH v5] erofs: introduce error injection infrastructure

[Chao Yu]

On 2018/7/1 2:39, Gao Xiang via Linux-erofs wrote:
> From: Chao Yu <yuchao0 at huawei.com>
> 
> This patch introduces error injection infrastructure, with it, we can
> inject error in any kernel exported common functions which erofs used,
> so that it can force erofs running into error paths, it turns out that
> tests can cover real rare paths more easily to find bugs.
> 
> Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
> Signed-off-by: Chao Yu <yuchao0 at huawei.com>
> ---
> v5 Gao Xiang <gaoxiang25 at huawei.com>:
> - fix implicit declaration of "kmalloc" by moving
>   <linux/slab.h> to internal.h
>   Reported-by: kbuild test robot <lkp at intel.com>
>   https://lists.01.org/pipermail/kbuild-all/2018-June/049644.html
> 
> v4 Gao Xiang <gaoxiang25 at huawei.com>:
> - avoid the global variable 'fault_name' to pollute symbol space.
>   Reported-by: kbuild test robot <lkp at intel.com>
>   https://lists.01.org/pipermail/kbuild-all/2018-June/049660.html

Thanks for fixing these issues.

Thanks,

> v3:
> - code rebase.
> v2:
> - add missing Kconfig entry.
> - fix double prefix 'erofs:' in erofs_show_injection_info macro.
> 
>  fs/erofs/Kconfig    |  6 +++++
>  fs/erofs/inode.c    |  4 ++--
>  fs/erofs/internal.h | 58 +++++++++++++++++++++++++++++++++++++++++++++
>  fs/erofs/super.c    | 41 ++++++++++++++++++++++++++++++--
>  fs/erofs/xattr.c    |  1 -
>  5 files changed, 105 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
> index 752f0e08d0b8..e9bab0f5600c 100644
> --- a/fs/erofs/Kconfig
> +++ b/fs/erofs/Kconfig
> @@ -86,3 +86,9 @@ config EROFS_FS_ZIP
>  
>  	  If you don't want to use compression, say N.
>  
> +config EROFS_FAULT_INJECTION
> +	bool "EROFS fault injection facility"
> +	depends on EROFS_FS
> +	help
> +	  Test EROFS to inject faults such as ENOMEM, EIO, and so on.
> +	  If unsure, say N.
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index 3dbce0cf41ff..844b999649f0 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -11,7 +11,6 @@
>   * distribution for more details.
>   */
>  #include "internal.h"
> -#include <linux/slab.h>
>  #include "xattr.h"
>  
>  /* no locking */
> @@ -100,6 +99,7 @@ static int read_inode(struct inode *inode, void *data)
>  int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
>  {
>  	struct erofs_vnode *vi = EROFS_V(inode);
> +	struct erofs_sb_info *sbi = EROFS_I_SB(inode);
>  	int mode = vi->data_mapping_mode;
>  
>  	BUG_ON(mode >= EROFS_INODE_LAYOUT_MAX);
> @@ -110,7 +110,7 @@ int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
>  
>  	/* fast symlink (following ext4) */
>  	if (S_ISLNK(inode->i_mode) && inode->i_size < PAGE_SIZE) {
> -		char *lnk = kmalloc(inode->i_size + 1, GFP_KERNEL);
> +		char *lnk = erofs_kmalloc(sbi, inode->i_size + 1, GFP_KERNEL);
>  
>  		if (unlikely(lnk == NULL))
>  			return -ENOMEM;
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 43620c07044d..2567fb83b5f6 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -23,6 +23,7 @@
>  #ifdef CONFIG_EROFS_FS_PAGE_BUNDLE
>  #include <linux/swap.h>
>  #endif
> +#include <linux/slab.h>
>  #include <linux/vmalloc.h>
>  #include "erofs_fs.h"
>  
> @@ -45,6 +46,22 @@
>  #define DBG_BUGON(...)  ((void)0)
>  #endif
>  
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> +enum {
> +	FAULT_KMALLOC,
> +	FAULT_MAX,
> +};
> +
> +extern char *erofs_fault_name[FAULT_MAX];
> +#define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
> +
> +struct erofs_fault_info {
> +	atomic_t inject_ops;
> +	unsigned int inject_rate;
> +	unsigned int inject_type;
> +};
> +#endif
> +
>  /* EROFS_SUPER_MAGIC_V1 to represent the whole file system */
>  #define EROFS_SUPER_MAGIC   EROFS_SUPER_MAGIC_V1
>  
> @@ -81,14 +98,55 @@ struct erofs_sb_info {
>  	char *dev_name;
>  
>  	unsigned int mount_opt;
> +
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> +	struct erofs_fault_info fault_info;	/* For fault injection */
> +#endif
>  };
>  
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> +#define erofs_show_injection_info(type)					\
> +	infoln("inject %s in %s of %pS", erofs_fault_name[type],        \
> +		__func__, __builtin_return_address(0))
> +
> +static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
> +{
> +	struct erofs_fault_info *ffi = &sbi->fault_info;
> +
> +	if (!ffi->inject_rate)
> +		return false;
> +
> +	if (!IS_FAULT_SET(ffi, type))
> +		return false;
> +
> +	atomic_inc(&ffi->inject_ops);
> +	if (atomic_read(&ffi->inject_ops) >= ffi->inject_rate) {
> +		atomic_set(&ffi->inject_ops, 0);
> +		return true;
> +	}
> +	return false;
> +}
> +#endif
> +
> +static inline void *erofs_kmalloc(struct erofs_sb_info *sbi,
> +					size_t size, gfp_t flags)
> +{
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> +	if (time_to_inject(sbi, FAULT_KMALLOC)) {
> +		erofs_show_injection_info(FAULT_KMALLOC);
> +		return NULL;
> +	}
> +#endif
> +	return kmalloc(size, flags);
> +}
> +
>  #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
>  #define EROFS_I_SB(inode) ((struct erofs_sb_info *)(inode)->i_sb->s_fs_info)
>  
>  /* Mount flags set via mount options or defaults */
>  #define EROFS_MOUNT_XATTR_USER		0x00000010
>  #define EROFS_MOUNT_POSIX_ACL		0x00000020
> +#define EROFS_MOUNT_FAULT_INJECTION	0x00000040
>  
>  #define clear_opt(sbi, option)	((sbi)->mount_opt &= ~EROFS_MOUNT_##option)
>  #define set_opt(sbi, option)	((sbi)->mount_opt |= EROFS_MOUNT_##option)
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 390a5217bbcc..d6046d4d787b 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -12,7 +12,6 @@
>   */
>  #include <linux/module.h>
>  #include <linux/buffer_head.h>
> -#include <linux/slab.h>
>  #include <linux/statfs.h>
>  #include <linux/seq_file.h>
>  #include <linux/parser.h>
> @@ -135,6 +134,26 @@ static int superblock_read(struct super_block *sb)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> +char *erofs_fault_name[FAULT_MAX] = {
> +	[FAULT_KMALLOC]		= "kmalloc",
> +};
> +
> +static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +						unsigned int rate)
> +{
> +	struct erofs_fault_info *ffi = &sbi->fault_info;
> +
> +	if (rate) {
> +		atomic_set(&ffi->inject_ops, 0);
> +		ffi->inject_rate = rate;
> +		ffi->inject_type = (1 << FAULT_MAX) - 1;
> +	} else {
> +		memset(ffi, 0, sizeof(struct erofs_fault_info));
> +	}
> +}
> +#endif
> +
>  static void default_options(struct erofs_sb_info *sbi)
>  {
>  #ifdef CONFIG_EROFS_FS_XATTR
> @@ -148,18 +167,21 @@ static void default_options(struct erofs_sb_info *sbi)
>  
>  enum {
>  	Opt_user_xattr,
> -	Opt_acl
> +	Opt_acl,
> +	Opt_fault_injection
>  };
>  
>  static match_table_t erofs_tokens = {
>  	{Opt_user_xattr, "user_xattr"},
>  	{Opt_acl, "acl"},
> +	{Opt_fault_injection, "fault_injection=%u"},
>  };
>  
>  static int parse_options(struct super_block *sb, char *options)
>  {
>  	substring_t args[MAX_OPT_ARGS];
>  	char *p;
> +	int arg = 0;
>  
>  	if (!options)
>  		return 0;
> @@ -192,6 +214,16 @@ static int parse_options(struct super_block *sb, char *options)
>  			infoln("acl options not supported");
>  			break;
>  #endif
> +		case Opt_fault_injection:
> +			if (args->from && match_int(args, &arg))
> +				return -EINVAL;
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> +			erofs_build_fault_attr(EROFS_SB(sb), arg);
> +			set_opt(EROFS_SB(sb), FAULT_INJECTION);
> +#else
> +			infoln("FAULT_INJECTION was not selected");
> +#endif
> +			break;
>  		default:
>  			infoln("Unrecognized mount option \"%s\" "
>  					"or missing value", p);
> @@ -451,6 +483,11 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>  		seq_puts(seq, ",acl");
>  	else
>  		seq_puts(seq, ",noacl");
> +#endif
> +#ifdef CONFIG_EROFS_FAULT_INJECTION
> +	if (test_opt(sbi, FAULT_INJECTION))
> +		seq_printf(seq, ",fault_injection=%u",
> +				sbi->fault_info.inject_rate);
>  #endif
>  	return 0;
>  }
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index fb8f3f1b3aab..aa0231768643 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -10,7 +10,6 @@
>   * License.  See the file COPYING in the main directory of the Linux
>   * distribution for more details.
>   */
> -#include <linux/slab.h>
>  #include <linux/security.h>
>  #include "xattr.h"
>  #include <linux/version.h>
> 

[PATCH v5] erofs: introduce error injection infrastructure

[Gao Xiang]

On 2018/7/1 11:56, Chao Yu wrote:
> Thanks for fixing these issues.
>
> Thanks,

;)

Thanks,
Gao Xiang

[PATCH 11/11] erofs: support tracepoint

[Chao Yu]

Add tracepoints for ->readpage{,s}, ->lookup.

Signed-off-by: Chao Yu <yuchao0 at huawei.com>
---
 fs/erofs/data.c              |   8 +++
 fs/erofs/namei.c             |   5 ++
 fs/erofs/super.c             |   2 +
 fs/erofs/unzip.c             |  10 +++-
 include/trace/events/erofs.h | 109 +++++++++++++++++++++++++++++++++++
 5 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 include/trace/events/erofs.h

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index e76de71c85ff..401b79a6ecd9 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -13,6 +13,9 @@
 #include "internal.h"
 #include <linux/prefetch.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/erofs.h>
+
 #if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
 static inline void read_endio(struct bio *bio, int err)
 #else
@@ -337,6 +340,8 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
 	erofs_off_t last_block;
 	struct bio *bio;
 
+	trace_erofs_readpage(page, true);
+
 	bio = erofs_read_raw_page(NULL, page->mapping,
 		page, &last_block, 1, false);
 
@@ -354,6 +359,9 @@ static int erofs_raw_access_readpages(struct file *filp,
 	erofs_off_t last_block;
 	struct bio *bio = NULL;
 	gfp_t gfp = readahead_gfp_mask(mapping);
+	struct page *page = list_last_entry(pages, struct page, lru);
+
+	trace_erofs_readpages(mapping->host, page, nr_pages, true);
 
 	for (; nr_pages; --nr_pages) {
 		struct page *page = list_entry(pages->prev, struct page, lru);
diff --git a/fs/erofs/namei.c b/fs/erofs/namei.c
index e30294b279d1..b701d9ba52a6 100644
--- a/fs/erofs/namei.c
+++ b/fs/erofs/namei.c
@@ -10,9 +10,12 @@
  * License.  See the file COPYING in the main directory of the Linux
  * distribution for more details.
  */
+
 #include "internal.h"
 #include "xattr.h"
 
+#include <trace/events/erofs.h>
+
 /* based on the value of qn->len is accurate */
 static inline int dirnamecmp(struct qstr *qn,
 	struct qstr *qd, unsigned *matched)
@@ -209,6 +212,8 @@ struct dentry *erofs_lookup(struct inode *dir,
 	/* dentry must be unhashed in lookup, no need to worry about */
 	BUG_ON(!d_unhashed(dentry));
 
+	trace_erofs_lookup(dir, dentry, flags);
+
 	/* file name exceeds fs limit */
 	if (unlikely(dentry->d_name.len > EROFS_NAME_LEN))
 		return ERR_PTR(-ENAMETOOLONG);
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index b5af35ec2797..003d8fccb3bd 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -18,6 +18,8 @@
 #include <linux/parser.h>
 #include "internal.h"
 
+#define CREATE_TRACE_POINTS
+
 static struct kmem_cache *erofs_inode_cachep __read_mostly;
 
 static void init_once(void *ptr)
diff --git a/fs/erofs/unzip.c b/fs/erofs/unzip.c
index 66ee5579d2c3..3f66c17e5171 100644
--- a/fs/erofs/unzip.c
+++ b/fs/erofs/unzip.c
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/blkdev.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_pack_cachep __read_mostly;
@@ -928,8 +929,11 @@ static int z_erofs_vle_normalaccess_readpage(struct file *file,
 		.sync = true
 	};
 	LIST_HEAD(pagepool);
+	int err;
+
+	trace_erofs_readpage(page, false);
 
-	int err = z_erofs_vle_do_read_page(page, &z_pvec,
+	err = z_erofs_vle_do_read_page(page, &z_pvec,
 		&z_iter, &m_iter, &pagepool, &collector);
 
 	if (z_iter.zip != NULL) {
@@ -1027,6 +1031,10 @@ static int z_erofs_vle_normalaccess_readpages(
 	struct address_space *mapping,
 	struct list_head *pages, unsigned nr_pages)
 {
+	struct page *page = list_last_entry(pages, struct page, lru);
+
+	trace_erofs_readpages(mapping->host, page, nr_pages, false);
+
 	return __z_erofs_vle_normalaccess_readpages(filp,
 		mapping, pages, nr_pages,
 		nr_pages < 4 /* sync */);
diff --git a/include/trace/events/erofs.h b/include/trace/events/erofs.h
new file mode 100644
index 000000000000..54d38282ba6a
--- /dev/null
+++ b/include/trace/events/erofs.h
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM erofs
+
+#if !defined(_TRACE_EROFS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_EROFS_H
+
+#include <linux/tracepoint.h>
+
+#define show_dev(dev)		MAJOR(dev), MINOR(dev)
+#define show_dev_ino(entry)	show_dev(entry->dev), (unsigned long)entry->ino
+
+#define show_file_type(type)						\
+	__print_symbolic(type,						\
+		{ 0,		"FILE" },				\
+		{ 1,		"DIR" })
+
+TRACE_EVENT(erofs_lookup,
+
+	TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
+
+	TP_ARGS(dir, dentry, flags),
+
+	TP_STRUCT__entry(
+		__field(dev_t,	dev)
+		__field(ino_t,	ino)
+		__field(const char *,	name)
+		__field(unsigned int, flags)
+	),
+
+	TP_fast_assign(
+		__entry->dev	= dir->i_sb->s_dev;
+		__entry->ino	= dir->i_ino;
+		__entry->name	= dentry->d_name.name;
+		__entry->flags	= flags;
+	),
+
+	TP_printk("dev = (%d,%d), pino = %lu, name:%s, flags:%u",
+		show_dev_ino(__entry),
+		__entry->name,
+		__entry->flags)
+);
+
+TRACE_EVENT(erofs_readpage,
+
+	TP_PROTO(struct page *page, bool raw),
+
+	TP_ARGS(page, raw),
+
+	TP_STRUCT__entry(
+		__field(dev_t,	dev)
+		__field(ino_t,	ino)
+		__field(int, dir)
+		__field(pgoff_t, index)
+		__field(int, uptodate)
+		__field(bool, raw)
+	),
+
+	TP_fast_assign(
+		__entry->dev	= page->mapping->host->i_sb->s_dev;
+		__entry->ino	= page->mapping->host->i_ino;
+		__entry->dir	= S_ISDIR(page->mapping->host->i_mode);
+		__entry->index	= page->index;
+		__entry->uptodate = PageUptodate(page);
+		__entry->raw = raw;
+	),
+
+	TP_printk("dev = (%d,%d), ino = %lu, %s, index = %lu, uptodate = %d "
+		"raw = %d",
+		show_dev_ino(__entry),
+		show_file_type(__entry->dir),
+		(unsigned long)__entry->index,
+		__entry->uptodate,
+		__entry->raw)
+);
+
+TRACE_EVENT(erofs_readpages,
+
+	TP_PROTO(struct inode *inode, struct page *page, unsigned int nrpage,
+		bool raw),
+
+	TP_ARGS(inode, page, nrpage, raw),
+
+	TP_STRUCT__entry(
+		__field(dev_t,	dev)
+		__field(ino_t,	ino)
+		__field(pgoff_t,	start)
+		__field(unsigned int,	nrpage)
+		__field(bool,		raw)
+	),
+
+	TP_fast_assign(
+		__entry->dev	= inode->i_sb->s_dev;
+		__entry->ino	= inode->i_ino;
+		__entry->start	= page->index;
+		__entry->nrpage	= nrpage;
+		__entry->raw	= raw;
+	),
+
+	TP_printk("dev = (%d,%d), ino = %lu, start = %lu nrpage = %u raw = %d",
+		show_dev_ino(__entry),
+		(unsigned long)__entry->start,
+		__entry->nrpage,
+		__entry->raw)
+);
+#endif /* _TRACE_EROFS_H */
+
+ /* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.18.0.rc1

[PATCH 11/11] erofs: support tracepoint

[Gao Xiang]

Hi Chao,

On 2018/6/22 10:01, Chao Yu wrote:
> Add tracepoints for ->readpage{,s}, ->lookup.
> 
> Signed-off-by: Chao Yu <yuchao0 at huawei.com> ---

Very useful feature (I could use it for the new unzip implementation :) )

>  fs/erofs/data.c              |   8 +++
>  fs/erofs/namei.c             |   5 ++
>  fs/erofs/super.c             |   2 +
>  fs/erofs/unzip.c             |  10 +++-
>  include/trace/events/erofs.h | 109 +++++++++++++++++++++++++++++++++++
>  5 files changed, 133 insertions(+), 1 deletion(-)
>  create mode 100644 include/trace/events/erofs.h
> 
> +	trace_erofs_readpage(page, true);
> +
>  	bio = erofs_read_raw_page(NULL, page->mapping,
>  		page, &last_block, 1, false);
>  
> @@ -354,6 +359,9 @@ static int erofs_raw_access_readpages(struct file *filp,
>  	erofs_off_t last_block;
>  	struct bio *bio = NULL;
>  	gfp_t gfp = readahead_gfp_mask(mapping);
> +	struct page *page = list_last_entry(pages, struct page, lru);
> +
> +	trace_erofs_readpages(mapping->host, page, nr_pages, true);
>  
>  	for (; nr_pages; --nr_pages) {
>  		struct page *page = list_entry(pages->prev, struct page, lru);

minor, how about remove the in-loop 'page' defination?

Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>

Thanks,
Gao Xiang

> diff --git a/fs/erofs/namei.c b/fs/erofs/namei.c
> index e30294b279d1..b701d9ba52a6 100644
> --- a/fs/erofs/namei.c
> +++ b/fs/erofs/namei.c

[PATCH 11/11] erofs: support tracepoint

[Chao Yu]

Hi Xiang,

On 2018/7/3 17:44, Gao Xiang wrote:
> Hi Chao,
> 
> On 2018/6/22 10:01, Chao Yu wrote:
>> Add tracepoints for ->readpage{,s}, ->lookup.
>>
>> Signed-off-by: Chao Yu <yuchao0 at huawei.com> ---
> 
> Very useful feature (I could use it for the new unzip implementation :) )

It seems we need more tracepoints for debug or accounting, but now I just add
basic ones, let's add more. ;)

> 
>>  fs/erofs/data.c              |   8 +++
>>  fs/erofs/namei.c             |   5 ++
>>  fs/erofs/super.c             |   2 +
>>  fs/erofs/unzip.c             |  10 +++-
>>  include/trace/events/erofs.h | 109 +++++++++++++++++++++++++++++++++++
>>  5 files changed, 133 insertions(+), 1 deletion(-)
>>  create mode 100644 include/trace/events/erofs.h
>>
>> +	trace_erofs_readpage(page, true);
>> +
>>  	bio = erofs_read_raw_page(NULL, page->mapping,
>>  		page, &last_block, 1, false);
>>  
>> @@ -354,6 +359,9 @@ static int erofs_raw_access_readpages(struct file *filp,
>>  	erofs_off_t last_block;
>>  	struct bio *bio = NULL;
>>  	gfp_t gfp = readahead_gfp_mask(mapping);
>> +	struct page *page = list_last_entry(pages, struct page, lru);
>> +
>> +	trace_erofs_readpages(mapping->host, page, nr_pages, true);
>>  
>>  	for (; nr_pages; --nr_pages) {
>>  		struct page *page = list_entry(pages->prev, struct page, lru);
> 
> minor, how about remove the in-loop 'page' defination?

Okay, let me remove this.

Thanks,

> 
> Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>
> 
> Thanks,
> Gao Xiang
> 
>> diff --git a/fs/erofs/namei.c b/fs/erofs/namei.c
>> index e30294b279d1..b701d9ba52a6 100644
>> --- a/fs/erofs/namei.c
>> +++ b/fs/erofs/namei.c

[PATCH 11/11] erofs: support tracepoint

[Gao Xiang]

On 2018/7/3 21:52, Chao Yu wrote:
> Hi Xiang,
> 
> On 2018/7/3 17:44, Gao Xiang wrote:
>> Hi Chao,
>>
>> On 2018/6/22 10:01, Chao Yu wrote:
>>> Add tracepoints for ->readpage{,s}, ->lookup.
>>>
>>> Signed-off-by: Chao Yu <yuchao0 at huawei.com> ---
>> Very useful feature (I could use it for the new unzip implementation :) )
> It seems we need more tracepoints for debug or accounting, but now I just add
> basic ones, let's add more. ;)
> 

I am transfering some useful map_blocks debug messages to tracepoints, which
seems more suitable :)

Thanks,
Gao Xiang