Re: [PATCH V2] mm/damon/dbgfs: Optimize target_ids interface write operation

[SeongJae Park <sj@kernel.org>]

On Fri, 22 Oct 2021 10:43:22 +0800 Xin Hao <xhao@linux.alibaba.com> wrote:

> 
> On 2021/10/22 上午1:30, SeongJae Park wrote:
> > Hello Xin,
> >
> > On Fri, 22 Oct 2021 00:44:16 +0800 Xin Hao <xhao@linux.alibaba.com> wrote:
> >
> >> When we want to clear previously set target ids,
> >> For example, it works as below now:
> >>      # echo 42 > target_ids
> >>      # cat target_ids
> >>      42
> >>      # echo > target_ids
> >>      # cat target_ids
> >>
> >> But in 'dbgfs_target_ids_write', there is no need to
> >> execute other codes, except call 'damon_set_targets'
> >> to clear previously set target ids. So there adds
> >> the 'nr_targets' judgment, if the value is 0, just
> >> call 'damon_set_targets', and then return.
> > It's true that it executes some unnecessary code.  However, I unsure if that is
> > a problem, as the code that will be additionally executed in this case are
> > quite simple ones, and therefore not supposed to incur viewable overhead.
> > After all, this is not a performance critical path.
> 
> Thank you for your detailed explanation. I may not describe it clearly, 
> making you think that i am making this
> 
> modification to improve performance，I just want to avoid irrelevant code 
> execution, thank you so much.

I guess I didn't make my point clear enough, sorry.  My concern in this patch
is the fact that it is adding more code.  IMHO, as the code is already working
correctly and benefit of this change is quite subtle as you also agreed, adding
the code here doesn't seem worthy but only making it harder to maintain, to me.

If I'm missing something, please let me know.


Thanks,
SJ

[...]