2021-10-14 02:05:57

by Rongwei Wang

[permalink] [raw]
Subject: [PATCH v2] mm/damon/dbgfs: remove unnecessary variables

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 <[email protected]>
---
mm/damon/dbgfs.c | 66 +++++++++++++++++++++++-------------------------
1 file changed, 31 insertions(+), 35 deletions(-)

diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
index 28d6abf27763..5cd4932800df 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,11 +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) {
if (targetid_is_pid(ctx))
dbgfs_put_pids(targets, nr_targets);
- ret = err;
+ } else {
+ ret = count;
}

unlock_out:
@@ -548,8 +547,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 +565,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 +636,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 +655,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 +681,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 +695,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


2021-10-14 07:23:41

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH v2] mm/damon/dbgfs: remove unnecessary variables

On Thu, 14 Oct 2021 10:03:33 +0800 Rongwei Wang <[email protected]> wrote:

> 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 <[email protected]>

Looks good to me. However, this doesn't cleanly applicable on latest -mm tree.
I will post a rebased version as a reply to this.


Thanks,
SJ

[...]

2021-10-14 07:32:37

by SeongJae Park

[permalink] [raw]
Subject: [PATCH v3] mm/damon/dbgfs: remove unnecessary variables

From: Rongwei Wang <[email protected]>

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 <[email protected]>
Signed-off-by: SeongJae Park <[email protected]>
---
Changes from v2
(https://lore.kernel.org/linux-mm/[email protected]/)
- Rebased on latest -mm tree (v5.15-rc5-mmots-2021-10-13-19-55)

mm/damon/dbgfs.c | 66 +++++++++++++++++++++++-------------------------
1 file changed, 31 insertions(+), 35 deletions(-)

diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
index 38188347d8ab..c90988a20fa4 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);
@@ -343,9 +342,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))
@@ -358,7 +356,7 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
scnprintf(kbuf, count, "42 ");
}

- targets = str_to_target_ids(nrs, ret, &nr_targets);
+ targets = str_to_target_ids(nrs, count, &nr_targets);
if (!targets) {
ret = -ENOMEM;
goto out;
@@ -393,11 +391,12 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
else
damon_pa_set_primitives(ctx);

- err = damon_set_targets(ctx, targets, nr_targets);
- if (err) {
+ ret = damon_set_targets(ctx, targets, nr_targets);
+ if (ret) {
if (id_is_pid)
dbgfs_put_pids(targets, nr_targets);
- ret = err;
+ } else {
+ ret = count;
}

unlock_out:
@@ -715,8 +714,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))
@@ -734,9 +732,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:
@@ -805,8 +803,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);
@@ -825,9 +822,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:
@@ -851,9 +848,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))
@@ -866,14 +862,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.17.1