[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a23c6f23-cf6b-1833-5603-363c45df933f@linux.alibaba.com>
Date: Fri, 22 Oct 2021 10:43:22 +0800
From: Xin Hao <xhao@...ux.alibaba.com>
To: SeongJae Park <sj@...nel.org>
Cc: sjpark@...zon.de, akpm@...ux-foundation.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] mm/damon/dbgfs: Optimize target_ids interface write
operation
On 2021/10/22 上午1:30, SeongJae Park wrote:
> Hello Xin,
>
> On Fri, 22 Oct 2021 00:44:16 +0800 Xin Hao <xhao@...ux.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.
>
> Thanks,
> SJ
>
>> Signed-off-by: Xin Hao <xhao@...ux.alibaba.com>
>> ---
>> mm/damon/dbgfs.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
>> index a02cf6bee8e8..1d83f4138fad 100644
>> --- a/mm/damon/dbgfs.c
>> +++ b/mm/damon/dbgfs.c
>> @@ -362,6 +362,12 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
>> goto out;
>> }
>>
>> + if (!nr_targets) {
>> + /* remove targets with previously-set primitive */
>> + damon_set_targets(ctx, NULL, 0);
>> + goto free_targets_out;
>> + }
>> +
>> if (id_is_pid) {
>> for (i = 0; i < nr_targets; i++) {
>> targets[i] = (unsigned long)find_get_pid(
>> --
>> 2.31.0
--
Best Regards!
Xin Hao
Powered by blists - more mailing lists