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

[SeongJae Park <sj@kernel.org>]

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