Messages in this thread Patch in this message | | | Date | Thu, 14 Oct 2021 00:27:08 +0800 | Subject | Re: [PATCH] mm/damon/dbgfs: remove unnecessary variables | From | 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 >
| |