[PATCH] mm/damon: trace: print address range in hex

[PATCH] mm/damon: trace: print address range in hex

[Akinobu Mita]

Currently, the address ranges are displayed in decimal format in
tracepoints for DAMON, but hexadecimal format is easier to compare with
address ranges in /proc/<pid>/numa_maps.

Existing tools such as damo need to be able to handle changing the format
of address ranges, so display address ranges in hexadecimal with a "0x"
prefix.

Cc: SeongJae Park <sj@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 include/trace/events/damon.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
index da4bd9fd1162..8df59ef18660 100644
--- a/include/trace/events/damon.h
+++ b/include/trace/events/damon.h
@@ -41,7 +41,7 @@ TRACE_EVENT_CONDITION(damos_before_apply,
 		__entry->nr_regions = nr_regions;
 	),
 
-	TP_printk("ctx_idx=%u scheme_idx=%u target_idx=%lu nr_regions=%u %lu-%lu: %u %u",
+	TP_printk("ctx_idx=%u scheme_idx=%u target_idx=%lu nr_regions=%u 0x%lx-0x%lx: %u %u",
 			__entry->context_idx, __entry->scheme_idx,
 			__entry->target_idx, __entry->nr_regions,
 			__entry->start, __entry->end,
@@ -73,7 +73,7 @@ TRACE_EVENT(damon_aggregated,
 		__entry->age = r->age;
 	),
 
-	TP_printk("target_id=%lu nr_regions=%u %lu-%lu: %u %u",
+	TP_printk("target_id=%lu nr_regions=%u 0x%lx-0x%lx: %u %u",
 			__entry->target_id, __entry->nr_regions,
 			__entry->start, __entry->end,
 			__entry->nr_accesses, __entry->age)
-- 
2.34.1

[PATCH damo] _damo_records: support address range in hex format

[Akinobu Mita]

In preparation for changing the display format of address ranges in
tracepoints for DAMON, if the start and end addresses of an address range
have a "0x" prefix, convert them as hexadecimal numbers.

Cc: SeongJae Park <sj@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 src/_damo_records.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/_damo_records.py b/src/_damo_records.py
index 6e541a0..55d864c 100644
--- a/src/_damo_records.py
+++ b/src/_damo_records.py
@@ -251,7 +251,7 @@ def parse_damon_aggregated_perf_script_fields(fields):
     target_id = int(fields[5].split('=')[1])
     nr_regions = int(fields[6].split('=')[1])
 
-    start_addr, end_addr = [int(x) for x in fields[7][:-1].split('-')]
+    start_addr, end_addr = [int(x, 0) for x in fields[7][:-1].split('-')]
     nr_accesses = int(fields[8])
     if len(fields) == 10:
         age = int(fields[9])
@@ -288,7 +288,7 @@ def parse_damos_before_apply_perf_script_fields(fields):
     target_id = int(fields[7].split('=')[1])
     nr_regions = int(fields[8].split('=')[1])
 
-    start_addr, end_addr = [int(x) for x in fields[9][:-1].split('-')]
+    start_addr, end_addr = [int(x, 0) for x in fields[9][:-1].split('-')]
     nr_accesses = int(fields[10])
     age = int(fields[11])
     region = _damon.DamonRegion(start_addr, end_addr, nr_accesses,
-- 
2.34.1

Re: [PATCH damo] _damo_records: support address range in hex format

[SeongJae Park]

On Mon, 30 Dec 2024 22:42:21 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:

> In preparation for changing the display format of address ranges in
> tracepoints for DAMON, if the start and end addresses of an address range
> have a "0x" prefix, convert them as hexadecimal numbers.

Thank you for this patch.  However, I'm not yet convinced to the DAMON side
change.  Let's discuss this change after a discussion on the DAMON change is
finished.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: trace: print address range in hex

[SeongJae Park]

Hello Akinobu,

On Mon, 30 Dec 2024 22:42:20 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:

> Currently, the address ranges are displayed in decimal format in
> tracepoints for DAMON, but hexadecimal format is easier to compare with
> address ranges in /proc/<pid>/numa_maps.

I agree that it could be easier for the use case.  But, I think writing and
using a script converting formats for DAMON tracepoint output or numa_maps
could also be a way to resolve the issue.  Do you have any problem at using
such solution?

> 
> Existing tools such as damo need to be able to handle changing the format
> of address ranges,

Thank you for taking care of damo, too!

> so display address ranges in hexadecimal with a "0x"
> prefix.

This is a user-visible behavioral change.  I understand it is an improvement
for your use case, but could be a degradation for some use cases.  Also the
change could confuse or break old use cases.  I understand that 'damo' users
will not get such problems thanks to your work, but not every DAMON users use
'damo'.

So I'd like to add this change only if there is some critical issue or benefit
that we cannot fix or achieve without this change.  As of now, I don't see such
things since I think you could use some user-space tools for your use case.
Please let me know if there is something that I'm missing.


Thanks,
SJ

> 
> Cc: SeongJae Park <sj@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>  include/trace/events/damon.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
> index da4bd9fd1162..8df59ef18660 100644
> --- a/include/trace/events/damon.h
> +++ b/include/trace/events/damon.h
> @@ -41,7 +41,7 @@ TRACE_EVENT_CONDITION(damos_before_apply,
>  		__entry->nr_regions = nr_regions;
>  	),
>  
> -	TP_printk("ctx_idx=%u scheme_idx=%u target_idx=%lu nr_regions=%u %lu-%lu: %u %u",
> +	TP_printk("ctx_idx=%u scheme_idx=%u target_idx=%lu nr_regions=%u 0x%lx-0x%lx: %u %u",
>  			__entry->context_idx, __entry->scheme_idx,
>  			__entry->target_idx, __entry->nr_regions,
>  			__entry->start, __entry->end,
> @@ -73,7 +73,7 @@ TRACE_EVENT(damon_aggregated,
>  		__entry->age = r->age;
>  	),
>  
> -	TP_printk("target_id=%lu nr_regions=%u %lu-%lu: %u %u",
> +	TP_printk("target_id=%lu nr_regions=%u 0x%lx-0x%lx: %u %u",
>  			__entry->target_id, __entry->nr_regions,
>  			__entry->start, __entry->end,
>  			__entry->nr_accesses, __entry->age)
> -- 
> 2.34.1

Re: [PATCH] mm/damon: trace: print address range in hex

[SeongJae Park]

On Mon, 30 Dec 2024 10:28:13 -0800 SeongJae Park <sj@kernel.org> wrote:

> Hello Akinobu,
> 
> On Mon, 30 Dec 2024 22:42:20 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:
> 
> > Currently, the address ranges are displayed in decimal format in
> > tracepoints for DAMON, but hexadecimal format is easier to compare with
> > address ranges in /proc/<pid>/numa_maps.
> 
> I agree that it could be easier for the use case.  But, I think writing and
> using a script converting formats for DAMON tracepoint output or numa_maps
> could also be a way to resolve the issue.

For example, we can extend 'damo' to optionally do the conversion.  Depending
on your use case (if you also use 'damo' for your use case), it might be a way
to move forward?


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: trace: print address range in hex

[Akinobu Mita]

2024年12月31日(火) 3:39 SeongJae Park <sj@kernel.org>:
>
> On Mon, 30 Dec 2024 10:28:13 -0800 SeongJae Park <sj@kernel.org> wrote:
>
> > Hello Akinobu,
> >
> > On Mon, 30 Dec 2024 22:42:20 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:
> >
> > > Currently, the address ranges are displayed in decimal format in
> > > tracepoints for DAMON, but hexadecimal format is easier to compare with
> > > address ranges in /proc/<pid>/numa_maps.
> >
> > I agree that it could be easier for the use case.  But, I think writing and
> > using a script converting formats for DAMON tracepoint output or numa_maps
> > could also be a way to resolve the issue.
>
> For example, we can extend 'damo' to optionally do the conversion.  Depending
> on your use case (if you also use 'damo' for your use case), it might be a way
> to move forward?

I'm currently using perf trace (-e damon:* --libtraceevent_print) to see which
address range accesses were detected or not, so as you suggested, I'll create a
script to convert the output of the perf trace.

Re: [PATCH] mm/damon: trace: print address range in hex

[SeongJae Park]

On Tue, 31 Dec 2024 21:56:43 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:

> 2024年12月31日(火) 3:39 SeongJae Park <sj@kernel.org>:
> >
> > On Mon, 30 Dec 2024 10:28:13 -0800 SeongJae Park <sj@kernel.org> wrote:
> >
> > > Hello Akinobu,
> > >
> > > On Mon, 30 Dec 2024 22:42:20 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:
> > >
> > > > Currently, the address ranges are displayed in decimal format in
> > > > tracepoints for DAMON, but hexadecimal format is easier to compare with
> > > > address ranges in /proc/<pid>/numa_maps.
> > >
> > > I agree that it could be easier for the use case.  But, I think writing and
> > > using a script converting formats for DAMON tracepoint output or numa_maps
> > > could also be a way to resolve the issue.
> >
> > For example, we can extend 'damo' to optionally do the conversion.  Depending
> > on your use case (if you also use 'damo' for your use case), it might be a way
> > to move forward?
> 
> I'm currently using perf trace (-e damon:* --libtraceevent_print) to see which
> address range accesses were detected or not,

Thank you for kindly sharing your use case.  I'm not very sure why you need to
use 'perf trace' instead of 'damo', though.  'damo' supports recording the
whole events and getting access monitoring result snapshots in live, with a few
results filtering criterias.  I just assume you had to use 'perf trace' since
you want to read full events in realtime, but please let me know if you want to
know more details about 'damo' features that might help you.

> so as you suggested, I'll create a
> script to convert the output of the perf trace.

Thank you for accepting my suggestion.  Please let me know if there is anything
that I can help at writing the script.  For example, if you really need to use
'perf trace', we're open to extend 'damo' for that.


Thanks,
SJ