[PATCH] mm/damon/dbgfs: remove unnecessary variables

[PATCH] mm/damon/dbgfs: remove unnecessary variables

[Rongwei Wang]

In some functions, it's unnecessary to declare 'err'
and 'ret' variables at the same time. This patch mainly
to simplify the issue of such declarations by reusing
one variable.

Signed-off-by: Rongwei Wang <rongwei.wang@linux.alibaba.com>
---
 mm/damon/dbgfs.c | 67 ++++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
index 28d6abf27763..d8dd68f72827 100644
--- a/mm/damon/dbgfs.c
+++ b/mm/damon/dbgfs.c
@@ -69,8 +69,7 @@ static ssize_t dbgfs_attrs_write(struct file *file,
 	struct damon_ctx *ctx = file->private_data;
 	unsigned long s, a, r, minr, maxr;
 	char *kbuf;
-	ssize_t ret = count;
-	int err;
+	ssize_t ret;
 
 	kbuf = user_input_str(buf, count, ppos);
 	if (IS_ERR(kbuf))
@@ -88,9 +87,9 @@ static ssize_t dbgfs_attrs_write(struct file *file,
 		goto unlock_out;
 	}
 
-	err = damon_set_attrs(ctx, s, a, r, minr, maxr);
-	if (err)
-		ret = err;
+	ret = damon_set_attrs(ctx, s, a, r, minr, maxr);
+	if (!ret)
+		ret = count;
 unlock_out:
 	mutex_unlock(&ctx->kdamond_lock);
 out:
@@ -220,14 +219,13 @@ static ssize_t dbgfs_schemes_write(struct file *file, const char __user *buf,
 	struct damon_ctx *ctx = file->private_data;
 	char *kbuf;
 	struct damos **schemes;
-	ssize_t nr_schemes = 0, ret = count;
-	int err;
+	ssize_t nr_schemes = 0, ret;
 
 	kbuf = user_input_str(buf, count, ppos);
 	if (IS_ERR(kbuf))
 		return PTR_ERR(kbuf);
 
-	schemes = str_to_schemes(kbuf, ret, &nr_schemes);
+	schemes = str_to_schemes(kbuf, count, &nr_schemes);
 	if (!schemes) {
 		ret = -EINVAL;
 		goto out;
@@ -239,11 +237,12 @@ static ssize_t dbgfs_schemes_write(struct file *file, const char __user *buf,
 		goto unlock_out;
 	}
 
-	err = damon_set_schemes(ctx, schemes, nr_schemes);
-	if (err)
-		ret = err;
-	else
+	ret = damon_set_schemes(ctx, schemes, nr_schemes);
+	if (!ret) {
+		ret = count;
 		nr_schemes = 0;
+	}
+
 unlock_out:
 	mutex_unlock(&ctx->kdamond_lock);
 	free_schemes_arr(schemes, nr_schemes);
@@ -342,9 +341,8 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
 	char *kbuf, *nrs;
 	unsigned long *targets;
 	ssize_t nr_targets;
-	ssize_t ret = count;
+	ssize_t ret;
 	int i;
-	int err;
 
 	kbuf = user_input_str(buf, count, ppos);
 	if (IS_ERR(kbuf))
@@ -352,7 +350,7 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
 
 	nrs = kbuf;
 
-	targets = str_to_target_ids(nrs, ret, &nr_targets);
+	targets = str_to_target_ids(nrs, count, &nr_targets);
 	if (!targets) {
 		ret = -ENOMEM;
 		goto out;
@@ -378,12 +376,12 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
 		goto unlock_out;
 	}
 
-	err = damon_set_targets(ctx, targets, nr_targets);
-	if (err) {
+	ret = damon_set_targets(ctx, targets, nr_targets);
+	if (ret < 0) {
 		if (targetid_is_pid(ctx))
 			dbgfs_put_pids(targets, nr_targets);
-		ret = err;
-	}
+	} else
+		ret = count;
 
 unlock_out:
 	mutex_unlock(&ctx->kdamond_lock);
@@ -548,8 +546,7 @@ static ssize_t dbgfs_mk_context_write(struct file *file,
 {
 	char *kbuf;
 	char *ctx_name;
-	ssize_t ret = count;
-	int err;
+	ssize_t ret;
 
 	kbuf = user_input_str(buf, count, ppos);
 	if (IS_ERR(kbuf))
@@ -567,9 +564,9 @@ static ssize_t dbgfs_mk_context_write(struct file *file,
 	}
 
 	mutex_lock(&damon_dbgfs_lock);
-	err = dbgfs_mk_context(ctx_name);
-	if (err)
-		ret = err;
+	ret = dbgfs_mk_context(ctx_name);
+	if (!ret)
+		ret = count;
 	mutex_unlock(&damon_dbgfs_lock);
 
 out:
@@ -638,8 +635,7 @@ static ssize_t dbgfs_rm_context_write(struct file *file,
 		const char __user *buf, size_t count, loff_t *ppos)
 {
 	char *kbuf;
-	ssize_t ret = count;
-	int err;
+	ssize_t ret;
 	char *ctx_name;
 
 	kbuf = user_input_str(buf, count, ppos);
@@ -658,9 +654,9 @@ static ssize_t dbgfs_rm_context_write(struct file *file,
 	}
 
 	mutex_lock(&damon_dbgfs_lock);
-	err = dbgfs_rm_context(ctx_name);
-	if (err)
-		ret = err;
+	ret = dbgfs_rm_context(ctx_name);
+	if (!ret)
+		ret = count;
 	mutex_unlock(&damon_dbgfs_lock);
 
 out:
@@ -684,9 +680,8 @@ static ssize_t dbgfs_monitor_on_read(struct file *file,
 static ssize_t dbgfs_monitor_on_write(struct file *file,
 		const char __user *buf, size_t count, loff_t *ppos)
 {
-	ssize_t ret = count;
+	ssize_t ret;
 	char *kbuf;
-	int err;
 
 	kbuf = user_input_str(buf, count, ppos);
 	if (IS_ERR(kbuf))
@@ -699,14 +694,14 @@ static ssize_t dbgfs_monitor_on_write(struct file *file,
 	}
 
 	if (!strncmp(kbuf, "on", count))
-		err = damon_start(dbgfs_ctxs, dbgfs_nr_ctxs);
+		ret = damon_start(dbgfs_ctxs, dbgfs_nr_ctxs);
 	else if (!strncmp(kbuf, "off", count))
-		err = damon_stop(dbgfs_ctxs, dbgfs_nr_ctxs);
+		ret = damon_stop(dbgfs_ctxs, dbgfs_nr_ctxs);
 	else
-		err = -EINVAL;
+		ret = -EINVAL;
 
-	if (err)
-		ret = err;
+	if (!ret)
+		ret = count;
 	kfree(kbuf);
 	return ret;
 }
-- 
2.27.0

Re: [PATCH] mm/damon/dbgfs: remove unnecessary variables

[SeongJae Park]

Hi Rongwei,


Thank you for this patch!  Looks good to me overall.  I left a couple of
nitpicks below, though.

On Wed, 13 Oct 2021 21:09:01 +0800 Rongwei Wang <rongwei.wang@linux.alibaba.com> wrote:

[...]
> @@ -352,7 +350,7 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
>  
>  	nrs = kbuf;
>  
> -	targets = str_to_target_ids(nrs, ret, &nr_targets);
> +	targets = str_to_target_ids(nrs, count, &nr_targets);
>  	if (!targets) {
>  		ret = -ENOMEM;
>  		goto out;
> @@ -378,12 +376,12 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
>  		goto unlock_out;
>  	}
>  
> -	err = damon_set_targets(ctx, targets, nr_targets);
> -	if (err) {
> +	ret = damon_set_targets(ctx, targets, nr_targets);
> +	if (ret < 0) {

I'd prefer 'if (ret) {', to be consistent with other part.

>  		if (targetid_is_pid(ctx))
>  			dbgfs_put_pids(targets, nr_targets);
> -		ret = err;
> -	}
> +	} else
> +		ret = count;

I'd prefer this to have braces:
https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces

>  
>  unlock_out:
>  	mutex_unlock(&ctx->kdamond_lock);
> @@ -548,8 +546,7 @@ static ssize_t dbgfs_mk_context_write(struct file *file,
[...]


Thanks,
SJ

Re: [PATCH] mm/damon/dbgfs: remove unnecessary variables

[Rongwei Wang]

On 10/13/21 11:04 PM, SeongJae Park wrote:
> Hi Rongwei,
> 
> 
> Thank you for this patch!  Looks good to me overall.  I left a couple of
> nitpicks below, though.
> 
> On Wed, 13 Oct 2021 21:09:01 +0800 Rongwei Wang <rongwei.wang@linux.alibaba.com> wrote:
> 
> [...]
>> @@ -352,7 +350,7 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
>>   
>>   	nrs = kbuf;
>>   
>> -	targets = str_to_target_ids(nrs, ret, &nr_targets);
>> +	targets = str_to_target_ids(nrs, count, &nr_targets);
>>   	if (!targets) {
>>   		ret = -ENOMEM;
>>   		goto out;
>> @@ -378,12 +376,12 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
>>   		goto unlock_out;
>>   	}
>>   
>> -	err = damon_set_targets(ctx, targets, nr_targets);
>> -	if (err) {
>> +	ret = damon_set_targets(ctx, targets, nr_targets);
>> +	if (ret < 0) {
> 
> I'd prefer 'if (ret) {', to be consistent with other part.
Hi

In fact, explicit style, such as 'if (ret < 0)', may be more 
understandable. But I agree with your advice that to be consistent
with other part. And I will update it like your said.
> 
>>   		if (targetid_is_pid(ctx))
>>   			dbgfs_put_pids(targets, nr_targets);
>> -		ret = err;
>> -	}
>> +	} else
>> +		ret = count;
> 
> I'd prefer this to have braces:
> https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces
Agree! I will update here.

Thanks!
> 
>>   
>>   unlock_out:
>>   	mutex_unlock(&ctx->kdamond_lock);
>> @@ -548,8 +546,7 @@ static ssize_t dbgfs_mk_context_write(struct file *file,
> [...]
> 
> 
> Thanks,
> SJ
>