Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   SeongJae Park <sj@kernel.org>
To:     xhao@linux.alibaba.com
Cc:     SeongJae Park <sj@kernel.org>, akpm@linux-foundation.org,
        corbet@lwn.net, skhan@linuxfoundation.org, rientjes@google.com,
        gregkh@linuxfoundation.org, linux-damon@amazon.com,
        linux-mm@kvack.org, linux-doc@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 05/13] mm/damon/sysfs: Support the physical address space monitoring
Date:   Mon, 28 Feb 2022 16:25:14 +0000
Message-Id: <20220228162514.4111-1-sj@kernel.org>
MIME-Version: 1.0
In-Reply-To: <d4b7fe7b-a908-989c-e86b-2ed404372d78@linux.alibaba.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 1 Mar 2022 00:11:12 +0800 xhao@linux.alibaba.com wrote:

> 
> On 2/28/22 4:13 PM, SeongJae Park wrote:
> > This commit makes DAMON sysfs interface supports the physical address
> > space monitoring.  Specifically, this commit adds support of the initial
> > monitoring regions set feature by adding 'regions' directory under each
> > target directory and makes context operations file to receive 'paddr' in
> > addition to 'vaddr'.
> >
> > As a result, the files hierarchy becomes as below:
> >
> >      /sys/kernel/mm/damon/admin
> >      │ kdamonds/nr_kdamonds
> >      │ │ 0/state,pid
> >      │ │ │ contexts/nr_contexts
> >      │ │ │ │ 0/operations
> >      │ │ │ │ │ monitoring_attrs/
> >      │ │ │ │ │ │ intervals/sample_us,aggr_us,update_us
> >      │ │ │ │ │ │ nr_regions/min,max
> >      │ │ │ │ │ targets/nr_targets
> >      │ │ │ │ │ │ 0/pid_target
> >      │ │ │ │ │ │ │ regions/nr_regions    <- NEW DIRECTORY
> >      │ │ │ │ │ │ │ │ 0/start,end
> >      │ │ │ │ │ │ │ │ ...
> >      │ │ │ │ │ │ ...
> >      │ │ │ │ ...
> >      │ │ ...
> >
> > Signed-off-by: SeongJae Park <sj@kernel.org>
> > ---
> >   mm/damon/sysfs.c | 276 ++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 271 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index 9221c93db6cc..968a4ba8e81b 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> > @@ -113,12 +113,220 @@ static struct kobj_type damon_sysfs_ul_range_ktype = {
> >   	.default_groups = damon_sysfs_ul_range_groups,
> >   };
> >   
> > +/*
> > + * init region directory
> > + */
> > +
> > +struct damon_sysfs_region {
> > +	struct kobject kobj;
> > +	unsigned long start;
> > +	unsigned long end;
> > +};
> > +
> > +static struct damon_sysfs_region *damon_sysfs_region_alloc(
> > +		unsigned long start,
> > +		unsigned long end)
> > +{
> > +	struct damon_sysfs_region *region = kmalloc(sizeof(*region),
> > +			GFP_KERNEL);
> > +
> > +	if (!region)
> > +		return NULL;
> > +	region->kobj = (struct kobject){};
> > +	region->start = start;
> > +	region->end = end;
> > +	return region;
> > +}
> > +
> 
> The interface "start" and "end" have the same problems
> 
> [root@rt2k03395 0]# echo 100 > start
> [root@rt2k03395 0]# echo 10 > end
> [root@rt2k03395 0]# cat end
> 10
> [root@rt2k03395 0]# cat start
> 100

As mentioned on the above answer[1], this is an intended behavior, so I don't
think a fix is needed.

[1] https://lore.kernel.org/linux-mm/20220228162318.4046-1-sj@kernel.org/


Thanks,
SJ

[...]
> -- 
> Best Regards!
> Xin Hao
>