Re: [PATCH V1 2/2] mm/damon: Modify the display form of damon tracepoint

[SeongJae Park <sj@kernel.org>]

Hi Xin,

On Fri, 10 Dec 2021 11:36:23 +0800 Xin Hao <xhao@linux.alibaba.com> wrote:

> [-- Attachment #1: Type: text/plain, Size: 4038 bytes --]
> 
> Hi SeongJae：
> 
> On 12/10/21 12:46 AM, SeongJae Park wrote:
> > On Fri, 10 Dec 2021 00:33:17 +0800 Xin Hao <xhao@linux.alibaba.com> wrote:
> >
> >> When I use the perf command to record damon monitor data, like below.
> >>      # perf record -e damon:damon_aggregated
> >>      # perf script
> >>      ...target_id=18446462667479739520 nr_regions=13 281472805928960-281472942936064...
> >>      ...target_id=18446462667479739520 nr_regions=13 281472942936064-281473080008704...
> >>      ...target_id=18446462667479739520 nr_regions=13 281473080008704-281473216634880...
> >>
> >>  From a user's point of view, the 'target_id' and 'damon_region' which displays in decimal
> >> are not very friendly, So there do some changes, keep the 'target_id' display consistent
> >> with 'dbgfs/target_ids' interface and 'damon_region' is displayed in hexadecimal, just like
> >> below.
> >>      # perf record -e damon:damon_aggregated
> >>      # perf script
> >>      ...target_id=5522 nr_regions=14 ffff716a3000-ffff79893000...
> >>      ...target_id=5522 nr_regions=14 ffff79893000-ffff819dc000...
> >>      ...target_id=5522 nr_regions=14 ffff819dc000-ffff89bd9000...
> >>
> >> Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
> >> ---
> >>   include/trace/events/damon.h | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
> >> index 99ffa601e351..67de51814f4c 100644
> >> --- a/include/trace/events/damon.h
> >> +++ b/include/trace/events/damon.h
[...]
> >>   	TP_fast_assign(
> >> -		__entry->target_id = t->id;
> >> +		__entry->target_id = (int)pid_vnr((struct pid *)t->id);
> > I think this would break physical address space monitoring.  Have you tested
> > this change for that?
> 
> Yes, you are right,  But I encountered some problems while testing 
> physical address, it seems that my operation did not work
> 
> I did some test like this:
> 
>      # echo "42 0x0000000840000000 0x000000103fffffff" > init_regions
> 
>      # echo paddr > target_ids
> 
>      # echo on > monitor_on
> 
> i get the physical address from my kernel startup log.
> 
> 15 [ 0.000000] Early memory node ranges
> 16 [ 0.000000] node 0: [mem 0x0000000040000000-0x000000083bc7ffff]
> 17 [ 0.000000] node 0: [mem 0x000000083bc80000-0x000000083bffffff]
> 18 [ 0.000000] node 0: [mem 0x000000083c000000-0x000000083c03ffff]
> 19 [ 0.000000] node 0: [mem 0x000000083c040000-0x000000083c0fffff]
> 20 [ 0.000000] node 0: [mem 0x000000083c100000-0x000000083f3dffff]
> 21 [ 0.000000] node 0: [mem 0x000000083f3e0000-0x000000083f46ffff]
> 22 [ 0.000000] node 0: [mem 0x000000083f470000-0x000000083f47ffff]
> 23 [ 0.000000] node 0: [mem 0x000000083f480000-0x000000083f59ffff]
> 24 [ 0.000000] node 0: [mem 0x000000083f5a0000-0x000000083fffffff]
> 25 [ 0.000000] node 1: [mem 0x0000000840000000-0x000000103fffffff]
> 26 [ 0.000000] node 2: [mem 0x0000001040000000-0x000000183fffffff]
> 27 [ 0.000000] node 3: [mem 0x0000001840000000-0x000000203fffffff]
> 28 [ 0.000000] Initmem setup node 0 [mem 
> 0x0000000040000000-0x000000083fffffff]
> 29 [ 0.000000] On node 0 totalpages: 8388608
> 
> Is there anything wrong ?

"The target id should already in target_ids file"[1].

For proper use of DAMON, I'd like to recommend you to refer to, or use the
official DAMON user space tool[2] instead of the debugfs interface.  As the
document[3] says "DAMON user space tool. This is for privileged people such as
system administrators who want a just-working human-friendly interface".

Also, some of your patches including this break the user space tool.  As it is
an important part of DAMON echosystem, it would be great if you could consider
taking care in keeping it unbroken, too.

[1] https://docs.kernel.org/admin-guide/mm/damon/usage.html#initial-monitoring-target-regions
[2] https://github.com/awslabs/damo
[3] https://docs.kernel.org/admin-guide/mm/damon/usage.html#detailed-usages


Thanks,
SJ

[...]