lkml.org 
[lkml]   [2021]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v3 2/2] mm/damon/dbgfs: Add adaptive_targets list check before enable monitor_on
Date

On 2021/10/20 下午4:12, SeongJae Park wrote:
> On Wed, 20 Oct 2021 07:21:21 +0000 SeongJae Park <sj@kernel.org> wrote:
>
>> On Wed, 20 Oct 2021 09:42:33 +0800 Xin Hao <xhao@linux.alibaba.com> wrote:
>>
>>> When the ctx->adaptive_targets list is empty,
>>> I did some test on monitor_on interface like this.
>>>
>>> # echo > /sys/kernel/debug/damon/target_ids
>> Thanks for the change, but you missed writing 'on' to 'monitor_on' in the above
>> example.
Sorry.
>>
>>> Though the ctx->adaptive_targets list is empty, but the
>>> kthread_run still be called, and the kdamond.x thread still
>>> be created, this is meaningless.
>>>
>>> So there adds a judgment in 'dbgfs_monitor_on_write',
>>> if the ctx->adaptive_targets list is empty, return -EINVAL.
>>>
>>> Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
>> Other parts looks good to me. However, seems this commit conflicts with a
>> patch[1] which already merged in -mm tree. Could you please rebase this on it?
>> FYI, all DAMON patches that merged in -mm tree are also applied on DAMON
>> development tree. So, you could get the patch applied tree by:
>>
>> $ git remote add sj git://git.kernel.org/pub/scm/linux/kernel/git/sj/linux.git
>> $ git fetch sj
>> $ git checkout 0fa378d04b12
>>
>> [1] https://lore.kernel.org/linux-mm/20211014073014.35754-1-sj@kernel.org/
Ok, i will correct it.
>>
>>
>> Thanks,
>> SJ
>>
>>> ---
>>> include/linux/damon.h | 1 +
>>> mm/damon/core.c | 5 +++++
>>> mm/damon/dbgfs.c | 13 ++++++++++---
>>> 3 files changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>>> index 715dadd21f7c..4fce5f1f6dad 100644
>>> --- a/include/linux/damon.h
>>> +++ b/include/linux/damon.h
>>> @@ -316,6 +316,7 @@ void damon_destroy_scheme(struct damos *s);
>>>
>>> struct damon_target *damon_new_target(unsigned long id);
>>> void damon_add_target(struct damon_ctx *ctx, struct damon_target *t);
>>> +bool damon_targets_empty(struct damon_ctx *ctx);
>>> void damon_free_target(struct damon_target *t);
>>> void damon_destroy_target(struct damon_target *t);
>>> unsigned int damon_nr_regions(struct damon_target *t);
>>> diff --git a/mm/damon/core.c b/mm/damon/core.c
>>> index 2f6785737902..c3a1374dbe0b 100644
>>> --- a/mm/damon/core.c
>>> +++ b/mm/damon/core.c
>>> @@ -156,6 +156,11 @@ void damon_add_target(struct damon_ctx *ctx, struct damon_target *t)
>>> list_add_tail(&t->list, &ctx->adaptive_targets);
>>> }
>>>
>>> +bool damon_targets_empty(struct damon_ctx *ctx)
>>> +{
>>> + return list_empty(&ctx->adaptive_targets);
>>> +}
>>> +
>>> static void damon_del_target(struct damon_target *t)
>>> {
>>> list_del(&t->list);
>>> diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
>>> index 38188347d8ab..9dee29f7d103 100644
>>> --- a/mm/damon/dbgfs.c
>>> +++ b/mm/damon/dbgfs.c
>>> @@ -865,12 +865,19 @@ static ssize_t dbgfs_monitor_on_write(struct file *file,
>>> return -EINVAL;
>>> }
>>>
>>> - if (!strncmp(kbuf, "on", count))
>>> + if (!strncmp(kbuf, "on", count)) {
>>> + int i;
>>> +
>>> + for (i = 0; i < dbgfs_nr_ctxs; i++) {
>>> + if (damon_targets_empty(dbgfs_ctxs[i]))
>>> + return -EINVAL;
> Oops, I forgot that you should 'kfree(kbuf)' before returning here. Please do
> that.
Okay,  i will correct it, thank you.
>
> Thanks,
> SJ
>
>>> + }
>>> err = damon_start(dbgfs_ctxs, dbgfs_nr_ctxs);
>>> - else if (!strncmp(kbuf, "off", count))
>>> + } else if (!strncmp(kbuf, "off", count)) {
>>> err = damon_stop(dbgfs_ctxs, dbgfs_nr_ctxs);
>>> - else
>>> + } else {
>>> err = -EINVAL;
>>> + }
>>>
>>> if (err)
>>> ret = err;
>>> --
>>> 2.31.0
>>
--
Best Regards!
Xin Hao

\
 
 \ /
  Last update: 2021-10-20 10:15    [W:0.035 / U:0.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site