| Message ID | 20220915142237.92529-1-xhao@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V1,1/2] mm/damon/sysfs: avoid call damon_target_has_pid() repeatedly | expand |
On Thu, 15 Sep 2022 22:22:36 +0800 Xin Hao <xhao@linux.alibaba.com> wrote: > In damon_sysfs_destroy_targets(), we call damon_target_has_pid() to > check whether the 'ctx' include a valid pid, but there no need to call > damon_target_has_pid() to check repeatedly, just need call it once. Good eyes, nice finding! > > Signed-off-by: Xin Hao <xhao@linux.alibaba.com> > --- > mm/damon/sysfs.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c > index 1fa0023f136e..966ea7892ccf 100644 > --- a/mm/damon/sysfs.c > +++ b/mm/damon/sysfs.c > @@ -2143,9 +2143,13 @@ static int damon_sysfs_set_attrs(struct damon_ctx *ctx, > static void damon_sysfs_destroy_targets(struct damon_ctx *ctx) > { > struct damon_target *t, *next; > + bool has_pid = false; > + > + if (damon_target_has_pid(ctx)) > + has_pid = true; How about doing more simple and short like below? bool has_pid = damon_target_has_pid(ctx) Other than this, Reviewed-by: SeongJae Park <sj@kernel.org> Thanks, SJ > > damon_for_each_target_safe(t, next, ctx) { > - if (damon_target_has_pid(ctx)) > + if (has_pid) > put_pid(t->pid); > damon_destroy_target(t); > } > -- > 2.31.0
在 2022/9/16 下午4:42, SeongJae Park 写道: > On Thu, 15 Sep 2022 22:22:36 +0800 Xin Hao <xhao@linux.alibaba.com> wrote: > >> In damon_sysfs_destroy_targets(), we call damon_target_has_pid() to >> check whether the 'ctx' include a valid pid, but there no need to call >> damon_target_has_pid() to check repeatedly, just need call it once. > Good eyes, nice finding! > >> Signed-off-by: Xin Hao <xhao@linux.alibaba.com> >> --- >> mm/damon/sysfs.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c >> index 1fa0023f136e..966ea7892ccf 100644 >> --- a/mm/damon/sysfs.c >> +++ b/mm/damon/sysfs.c >> @@ -2143,9 +2143,13 @@ static int damon_sysfs_set_attrs(struct damon_ctx *ctx, >> static void damon_sysfs_destroy_targets(struct damon_ctx *ctx) >> { >> struct damon_target *t, *next; >> + bool has_pid = false; >> + >> + if (damon_target_has_pid(ctx)) >> + has_pid = true; > How about doing more simple and short like below? > > bool has_pid = damon_target_has_pid(ctx) Yes, Do like this, make code look more clean. but this patch has been merged into, i send a new one to fix it ? > > Other than this, > > Reviewed-by: SeongJae Park <sj@kernel.org> > > > Thanks, > SJ > >> damon_for_each_target_safe(t, next, ctx) { >> - if (damon_target_has_pid(ctx)) >> + if (has_pid) >> put_pid(t->pid); >> damon_destroy_target(t); >> } >> -- >> 2.31.0
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c index 1fa0023f136e..966ea7892ccf 100644 --- a/mm/damon/sysfs.c +++ b/mm/damon/sysfs.c @@ -2143,9 +2143,13 @@ static int damon_sysfs_set_attrs(struct damon_ctx *ctx, static void damon_sysfs_destroy_targets(struct damon_ctx *ctx) { struct damon_target *t, *next; + bool has_pid = false; + + if (damon_target_has_pid(ctx)) + has_pid = true; damon_for_each_target_safe(t, next, ctx) { - if (damon_target_has_pid(ctx)) + if (has_pid) put_pid(t->pid); damon_destroy_target(t); }
In damon_sysfs_destroy_targets(), we call damon_target_has_pid() to check whether the 'ctx' include a valid pid, but there no need to call damon_target_has_pid() to check repeatedly, just need call it once. Signed-off-by: Xin Hao <xhao@linux.alibaba.com> --- mm/damon/sysfs.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) -- 2.31.0