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

[SeongJae Park <sj@kernel.org>]

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
> @@ -17,7 +17,7 @@ TRACE_EVENT(damon_aggregated,
>  	TP_ARGS(t, r, nr_regions),
> 
>  	TP_STRUCT__entry(
> -		__field(unsigned long, target_id)
> +		__field(int, target_id)
>  		__field(unsigned int, nr_regions)
>  		__field(unsigned long, start)
>  		__field(unsigned long, end)
> @@ -26,7 +26,7 @@ TRACE_EVENT(damon_aggregated,
>  	),
> 
>  	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?


Thanks,
SJ

>  		__entry->nr_regions = nr_regions;
>  		__entry->start = r->ar.start;
>  		__entry->end = r->ar.end;
> @@ -34,7 +34,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=%u nr_regions=%u %lx-%lx: %u %u",
>  			__entry->target_id, __entry->nr_regions,
>  			__entry->start, __entry->end,
>  			__entry->nr_accesses, __entry->age)
> --
> 2.31.0