[PATCH v2 0/6] staging: erofs: option validation for remount and some code cleanups

[PATCH v2 0/6] staging: erofs: option validation for remount and some code cleanups

[Chengguang Xu]

This patch set mainly does option validation for remount and at
the same time does related code cleanups. Currently when we call
fault injection related code we have to surround it with macro
CONFIG_EROFS_FAULT_INJECTION in every calling place, after this
patch set we don't have to do that, so the code looks clean and
more understandable.

v1->v2:
Address Chao's comments:
- Allow to set fault_injection=0.
- Keep flag bit when setting fault_injection=0.
- Show injection info in original place.
- Rebase code on latest erofs branch in Chao's linux tree.
- Fix building issue.


Chengguang Xu (6):
  staging: erofs: code cleanup for erofs_kmalloc()
  staging: erofs: code cleanup for option parsing of fault_injection
  staging: erofs: introduce a new helper __erofs_build_fault_attr()
  staging: erofs: introduce a new helper erofs_get_fault_rate()
  staging: erofs: code cleanup for erofs_show_options()
  staging: erofs: option validation in remount

 drivers/staging/erofs/internal.h | 13 ++++--
 drivers/staging/erofs/super.c    | 73 +++++++++++++++++++++++++-------
 2 files changed, 67 insertions(+), 19 deletions(-)

-- 
2.17.1

[PATCH v2 1/6] staging: erofs: code cleanup for erofs_kmalloc()

[Chengguang Xu]

Define a dummy function of time_to_inject()/erofs_show_injection_info(),
so that we don't have to check macro CONFIG_EROFS_FAULT_INJECTION in
calling place.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
---
 drivers/staging/erofs/internal.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index f20c6e9b7471..0011b9d505fd 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -42,12 +42,12 @@
 #define DBG_BUGON(...)          ((void)0)
 #endif
 
-#ifdef CONFIG_EROFS_FAULT_INJECTION
 enum {
 	FAULT_KMALLOC,
 	FAULT_MAX,
 };
 
+#ifdef CONFIG_EROFS_FAULT_INJECTION
 extern char *erofs_fault_name[FAULT_MAX];
 #define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
 
@@ -143,17 +143,24 @@ static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
 	}
 	return false;
 }
+#else
+static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
+{
+	return false;
+}
+
+static inline void erofs_show_injection_info(int type)
+{
+}
 #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);
 }
 
-- 
2.17.1

[PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection

[Chengguang Xu]

Define a dummpy function of erofs_build_fault_attr() when macro
CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
check the macro in calling place. Based on above adjustment,
do proper code cleanup for option parsing of fault_injection.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
---
 drivers/staging/erofs/super.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 9e421536cbdf..7ce2fd3d49f3 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
 	[FAULT_KMALLOC]		= "kmalloc",
 };
 
-static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
-						unsigned int rate)
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+					substring_t *args)
 {
 	struct erofs_fault_info *ffi = &sbi->fault_info;
+	int rate = 0;
+
+	if (args->from && match_int(args, &rate))
+		return -EINVAL;
 
 	if (rate) {
 		atomic_set(&ffi->inject_ops, 0);
@@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
 	} else {
 		memset(ffi, 0, sizeof(struct erofs_fault_info));
 	}
+
+	set_opt(sbi, FAILt_INJECTION);
+}
+#else
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+					substring_t *args)
+{
+	infoln("fault_injection options not supported");
+	return 0;
 }
 #endif
 
@@ -193,7 +206,7 @@ static int parse_options(struct super_block *sb, char *options)
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *p;
-	int arg = 0;
+	int err;
 
 	if (!options)
 		return 0;
@@ -238,18 +251,12 @@ static int parse_options(struct super_block *sb, char *options)
 			infoln("noacl options not supported");
 			break;
 #endif
-#ifdef CONFIG_EROFS_FAULT_INJECTION
-		case Opt_fault_injection:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
-			erofs_build_fault_attr(EROFS_SB(sb), arg);
-			set_opt(EROFS_SB(sb), FAULT_INJECTION);
-			break;
-#else
 		case Opt_fault_injection:
-			infoln("fault_injection options not supported");
+			err = erofs_build_fault_attr(EROFS_SB(sb), args);
+			if (err)
+				return err;
 			break;
-#endif
+
 		default:
 			errln("Unrecognized mount option \"%s\" "
 					"or missing value", p);
-- 
2.17.1

[PATCH v2 3/6] staging: erofs: introduce a new helper __erofs_build_fault_attr()

[Chengguang Xu]

Introduce a new helper __erofs_build_fault_attr() to handle set/clear
erofs_fault_info, we need this funciton for internal use case.
for example, reset fault_injection option in remount.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
---
 drivers/staging/erofs/super.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 7ce2fd3d49f3..3f5ae64b9c60 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,14 +145,10 @@ char *erofs_fault_name[FAULT_MAX] = {
 	[FAULT_KMALLOC]		= "kmalloc",
 };
 
-static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
-					substring_t *args)
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+					unsigned int rate)
 {
 	struct erofs_fault_info *ffi = &sbi->fault_info;
-	int rate = 0;
-
-	if (args->from && match_int(args, &rate))
-		return -EINVAL;
 
 	if (rate) {
 		atomic_set(&ffi->inject_ops, 0);
@@ -164,7 +160,24 @@ static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 
 	set_opt(sbi, FAILt_INJECTION);
 }
+
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+					substring_t *args)
+{
+	int rate = 0;
+
+	if (args->from && match_int(args, &rate))
+		return -EINVAL;
+
+	__erofs_build_fault_attr(sbi, rate);
+	return 0;
+}
 #else
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+					unsigned int rate)
+{
+}
+
 static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 					substring_t *args)
 {
-- 
2.17.1

[PATCH v2 4/6] staging: erofs: introduce a new helper erofs_get_fault_rate()

[Chengguang Xu]

Introduce a new helper for getting fault rate, so that we
don't have to check macro in calling place.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
---
 drivers/staging/erofs/super.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 3f5ae64b9c60..24f3423ed804 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,6 +145,11 @@ char *erofs_fault_name[FAULT_MAX] = {
 	[FAULT_KMALLOC]		= "kmalloc",
 };
 
+static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
+{
+	return sbi->fault_info.inject_rate;
+}
+
 static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
 					unsigned int rate)
 {
@@ -173,6 +178,11 @@ static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
 	return 0;
 }
 #else
+static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
+{
+	return 0;
+}
+
 static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
 					unsigned int rate)
 {
-- 
2.17.1

[PATCH v2 5/6] staging: erofs: code cleanup for erofs_show_options()

[Chengguang Xu]

Call erofs_get_fault_rate() to get fault rate instead of
directly getting it from sbi, so we can remove the macro
check surrounding it.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
---
 drivers/staging/erofs/super.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 24f3423ed804..a6a4874d9c9e 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -648,11 +648,9 @@ 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
+			erofs_get_fault_rate(sbi));
 	return 0;
 }
 
-- 
2.17.1

[PATCH v2 6/6] staging: erofs: option validation in remount

[Chengguang Xu]

Add option validation in remount. After this patch, remount
can change recognized options, and for unknown options remount
will fail and report error.

Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
---
 drivers/staging/erofs/super.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index a6a4874d9c9e..2a952cc7e27d 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -656,10 +656,23 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 
 static int erofs_remount(struct super_block *sb, int *flags, char *data)
 {
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	unsigned int org_mnt_opt = sbi->mount_opt;
+	unsigned int org_inject_rate = erofs_get_fault_rate(sbi);
+	int err;
+
 	BUG_ON(!sb_rdonly(sb));
+	err = parse_options(sb, data);
+	if (err)
+		goto out;
 
 	*flags |= MS_RDONLY;
 	return 0;
+out:
+	__erofs_build_fault_attr(sbi, org_inject_rate);
+	sbi->mount_opt = org_mnt_opt;
+
+	return err;
 }
 
 const struct super_operations erofs_sops = {
-- 
2.17.1

[PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection

[Gao Xiang]

Hi Chengguang,

On 2018/9/17 23:34, Chengguang Xu wrote:
> Define a dummpy function of erofs_build_fault_attr() when macro
> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
> check the macro in calling place. Based on above adjustment,
> do proper code cleanup for option parsing of fault_injection.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
> ---
>  drivers/staging/erofs/super.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 9e421536cbdf..7ce2fd3d49f3 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
>  	[FAULT_KMALLOC]		= "kmalloc",
>  };
>  
> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
> -						unsigned int rate)
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +					substring_t *args)
>  {
>  	struct erofs_fault_info *ffi = &sbi->fault_info;
> +	int rate = 0;
> +
> +	if (args->from && match_int(args, &rate))
> +		return -EINVAL;
>  
>  	if (rate) {
>  		atomic_set(&ffi->inject_ops, 0);
> @@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>  	} else {
>  		memset(ffi, 0, sizeof(struct erofs_fault_info));
>  	}
> +
> +	set_opt(sbi, FAILt_INJECTION);
drivers/staging/erofs/super.c: In function ?__erofs_build_fault_attr?:
drivers/staging/erofs/internal.h:176:51: error: ?EROFS_MOUNT_FAILt_INJECTION? undeclared (first use in this function)
 #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
                                                   ^
drivers/staging/erofs/super.c:166:2: note: in expansion of macro ?set_opt?
  set_opt(sbi, FAILt_INJECTION);
  ^
drivers/staging/erofs/internal.h:176:51: note: each undeclared identifier is reported only once for each function it appears in
 #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
                                                   ^
drivers/staging/erofs/super.c:166:2: note: in expansion of macro ?set_opt?
  set_opt(sbi, FAILt_INJECTION);
  ^

Thanks,
Gao Xiang


> +}
> +#else
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +					substring_t *args)
> +{
> +	infoln("fault_injection options not supported");
> +	return 0;
>  }
>  #endif
>  
> @@ -193,7 +206,7 @@ static int parse_options(struct super_block *sb, char *options)
>  {
>  	substring_t args[MAX_OPT_ARGS];
>  	char *p;
> -	int arg = 0;
> +	int err;
>  
>  	if (!options)
>  		return 0;
> @@ -238,18 +251,12 @@ static int parse_options(struct super_block *sb, char *options)
>  			infoln("noacl options not supported");
>  			break;
>  #endif
> -#ifdef CONFIG_EROFS_FAULT_INJECTION
> -		case Opt_fault_injection:
> -			if (args->from && match_int(args, &arg))
> -				return -EINVAL;
> -			erofs_build_fault_attr(EROFS_SB(sb), arg);
> -			set_opt(EROFS_SB(sb), FAULT_INJECTION);
> -			break;
> -#else
>  		case Opt_fault_injection:
> -			infoln("fault_injection options not supported");
> +			err = erofs_build_fault_attr(EROFS_SB(sb), args);
> +			if (err)
> +				return err;
>  			break;
> -#endif
> +
>  		default:
>  			errln("Unrecognized mount option \"%s\" "
>  					"or missing value", p);
> 

[PATCH v2 1/6] staging: erofs: code cleanup for erofs_kmalloc()

[Chao Yu]

On 2018/9/17 23:34, Chengguang Xu wrote:
> Define a dummy function of time_to_inject()/erofs_show_injection_info(),
> so that we don't have to check macro CONFIG_EROFS_FAULT_INJECTION in
> calling place.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>

Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Thanks,

[PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection

[Chao Yu]

On 2018/9/17 23:34, Chengguang Xu wrote:
> Define a dummpy function of erofs_build_fault_attr() when macro
> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
> check the macro in calling place. Based on above adjustment,
> do proper code cleanup for option parsing of fault_injection.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
> ---
>  drivers/staging/erofs/super.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 9e421536cbdf..7ce2fd3d49f3 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
>  	[FAULT_KMALLOC]		= "kmalloc",
>  };
>  
> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
> -						unsigned int rate)
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +					substring_t *args)
>  {
>  	struct erofs_fault_info *ffi = &sbi->fault_info;
> +	int rate = 0;
> +
> +	if (args->from && match_int(args, &rate))
> +		return -EINVAL;
>  
>  	if (rate) {
>  		atomic_set(&ffi->inject_ops, 0);
> @@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>  	} else {
>  		memset(ffi, 0, sizeof(struct erofs_fault_info));
>  	}
> +
> +	set_opt(sbi, FAILt_INJECTION);

As Xiang mentioned, it needs to fix this, otherwise, it looks good to me. :)

Thanks,

> +}
> +#else
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> +					substring_t *args)
> +{
> +	infoln("fault_injection options not supported");
> +	return 0;
>  }
>  #endif
>  
> @@ -193,7 +206,7 @@ static int parse_options(struct super_block *sb, char *options)
>  {
>  	substring_t args[MAX_OPT_ARGS];
>  	char *p;
> -	int arg = 0;
> +	int err;
>  
>  	if (!options)
>  		return 0;
> @@ -238,18 +251,12 @@ static int parse_options(struct super_block *sb, char *options)
>  			infoln("noacl options not supported");
>  			break;
>  #endif
> -#ifdef CONFIG_EROFS_FAULT_INJECTION
> -		case Opt_fault_injection:
> -			if (args->from && match_int(args, &arg))
> -				return -EINVAL;
> -			erofs_build_fault_attr(EROFS_SB(sb), arg);
> -			set_opt(EROFS_SB(sb), FAULT_INJECTION);
> -			break;
> -#else
>  		case Opt_fault_injection:
> -			infoln("fault_injection options not supported");
> +			err = erofs_build_fault_attr(EROFS_SB(sb), args);
> +			if (err)
> +				return err;
>  			break;
> -#endif
> +
>  		default:
>  			errln("Unrecognized mount option \"%s\" "
>  					"or missing value", p);
> 

[PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection

[cgxu519]

On 09/18/2018 03:07 PM, Gao Xiang wrote:
> Hi Chengguang,
>
> On 2018/9/17 23:34, Chengguang Xu wrote:
>> Define a dummpy function of erofs_build_fault_attr() when macro
>> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
>> check the macro in calling place. Based on above adjustment,
>> do proper code cleanup for option parsing of fault_injection.
>>
>> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
>> ---
>>   drivers/staging/erofs/super.c | 33 ++++++++++++++++++++-------------
>>   1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
>> index 9e421536cbdf..7ce2fd3d49f3 100644
>> --- a/drivers/staging/erofs/super.c
>> +++ b/drivers/staging/erofs/super.c
>> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
>>   	[FAULT_KMALLOC]		= "kmalloc",
>>   };
>>   
>> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>> -						unsigned int rate)
>> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>> +					substring_t *args)
>>   {
>>   	struct erofs_fault_info *ffi = &sbi->fault_info;
>> +	int rate = 0;
>> +
>> +	if (args->from && match_int(args, &rate))
>> +		return -EINVAL;
>>   
>>   	if (rate) {
>>   		atomic_set(&ffi->inject_ops, 0);
>> @@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>   	} else {
>>   		memset(ffi, 0, sizeof(struct erofs_fault_info));
>>   	}
>> +
>> +	set_opt(sbi, FAILt_INJECTION);
> drivers/staging/erofs/super.c: In function ?__erofs_build_fault_attr?:
> drivers/staging/erofs/internal.h:176:51: error: ?EROFS_MOUNT_FAILt_INJECTION? undeclared (first use in this function)
>   #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
>                                                     ^
> drivers/staging/erofs/super.c:166:2: note: in expansion of macro ?set_opt?
>    set_opt(sbi, FAILt_INJECTION);
>    ^
> drivers/staging/erofs/internal.h:176:51: note: each undeclared identifier is reported only once for each function it appears in
>   #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
>                                                     ^
> drivers/staging/erofs/super.c:166:2: note: in expansion of macro ?set_opt?
>    set_opt(sbi, FAILt_INJECTION);
>    ^

Hi Xiang,

I'm really sorry for that, I'm curious how it passed my building test.
I deleted all existing config and? binary files and tested with/without 
INJECTION config this time.


Thanks,
Chengguang

[PATCH v2 3/6] staging: erofs: introduce a new helper __erofs_build_fault_attr()

[Chao Yu]

On 2018/9/17 23:34, Chengguang Xu wrote:
> Introduce a new helper __erofs_build_fault_attr() to handle set/clear
> erofs_fault_info, we need this funciton for internal use case.
> for example, reset fault_injection option in remount.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>

Since this patch is related to your [PATCH 6/6], so I think they can be
merge to one, so the purpose of patch can be more clear. Anyway, it's not a
big deal. you can add:

Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Thanks,

[PATCH v2 4/6] staging: erofs: introduce a new helper erofs_get_fault_rate()

[Chao Yu]

On 2018/9/17 23:34, Chengguang Xu wrote:
> Introduce a new helper for getting fault rate, so that we
> don't have to check macro in calling place.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>

Can you fold this patch into [5/6]?

Thanks,

[PATCH v2 6/6] staging: erofs: option validation in remount

[Chao Yu]

On 2018/9/17 23:34, Chengguang Xu wrote:
> Add option validation in remount. After this patch, remount
> can change recognized options, and for unknown options remount
> will fail and report error.
> 
> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>

Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Thanks,

[PATCH v2 1/6] staging: erofs: code cleanup for erofs_kmalloc()

[Gao Xiang]

On 2018/9/18 18:41, Chao Yu wrote:
> On 2018/9/17 23:34, Chengguang Xu wrote:
>> Define a dummy function of time_to_inject()/erofs_show_injection_info(),
>> so that we don't have to check macro CONFIG_EROFS_FAULT_INJECTION in
>> calling place.
>>
>> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
> 
> Reviewed-by: Chao Yu <yuchao0 at huawei.com>
> 

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

Thanks,
Gao Xiang

> Thanks,
> 

[PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection

[Gao Xiang]

Hi Chengguang,

On 2018/9/18 18:47, cgxu519 wrote:
> On 09/18/2018 03:07 PM, Gao Xiang wrote:
>> Hi Chengguang,
>>
>> On 2018/9/17 23:34, Chengguang Xu wrote:
>>> Define a dummpy function of erofs_build_fault_attr() when macro
>>> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
>>> check the macro in calling place. Based on above adjustment,
>>> do proper code cleanup for option parsing of fault_injection.
>>>
>>> Signed-off-by: Chengguang Xu <cgxu519 at gmx.com>
>>> ---
>>> ? drivers/staging/erofs/super.c | 33 ++++++++++++++++++++-------------
>>> ? 1 file changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
>>> index 9e421536cbdf..7ce2fd3d49f3 100644
>>> --- a/drivers/staging/erofs/super.c
>>> +++ b/drivers/staging/erofs/super.c
>>> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
>>> ????? [FAULT_KMALLOC]??????? = "kmalloc",
>>> ? };
>>> ? -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>> -??????????????????????? unsigned int rate)
>>> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>> +??????????????????? substring_t *args)
>>> ? {
>>> ????? struct erofs_fault_info *ffi = &sbi->fault_info;
>>> +??? int rate = 0;
>>> +
>>> +??? if (args->from && match_int(args, &rate))
>>> +??????? return -EINVAL;
>>> ? ????? if (rate) {
>>> ????????? atomic_set(&ffi->inject_ops, 0);
>>> @@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>> ????? } else {
>>> ????????? memset(ffi, 0, sizeof(struct erofs_fault_info));
>>> ????? }
>>> +
>>> +??? set_opt(sbi, FAILt_INJECTION);
>> drivers/staging/erofs/super.c: In function ?__erofs_build_fault_attr?:
>> drivers/staging/erofs/internal.h:176:51: error: ?EROFS_MOUNT_FAILt_INJECTION? undeclared (first use in this function)
>> ? #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
>> ??????????????????????????????????????????????????? ^
>> drivers/staging/erofs/super.c:166:2: note: in expansion of macro ?set_opt?
>> ?? set_opt(sbi, FAILt_INJECTION);
>> ?? ^
>> drivers/staging/erofs/internal.h:176:51: note: each undeclared identifier is reported only once for each function it appears in
>> ? #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
>> ??????????????????????????????????????????????????? ^
>> drivers/staging/erofs/super.c:166:2: note: in expansion of macro ?set_opt?
>> ?? set_opt(sbi, FAILt_INJECTION);
>> ?? ^
> 
> Hi Xiang,
> 
> I'm really sorry for that, I'm curious how it passed my building test.
> I deleted all existing config and? binary files and tested with/without INJECTION config this time.

I have no idea either...
No worry about that, just resend your fixed patch.

Thanks,
Gao Xiang

> 
> 
> Thanks,
> Chengguang
> 
> 
>