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@xxxxxxxxxxxxxxxxx> 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