* [PATCH V1 0/2] mm/damon: Do little changes @ 2021-12-09 16:33 Xin Hao 2021-12-09 16:33 ` [PATCH V1 1/2] mm/damon/dbgfs: Avoid target_ids display wrong pid value Xin Hao 2021-12-09 16:33 ` [PATCH V1 2/2] mm/damon: Modify the display form of damon tracepoint Xin Hao 0 siblings, 2 replies; 4+ messages in thread From: Xin Hao @ 2021-12-09 16:33 UTC (permalink / raw) To: sj; +Cc: xhao, akpm, linux-mm, linux-kernel These patches are mainly used for fine adjustments. Xin Hao (2): mm/damon/dbgfs: Avoid target_ids display wrong pid value mm/damon: Modify the display form of damon tracepoint include/trace/events/damon.h | 6 +++--- mm/damon/dbgfs.c | 8 +++----- 2 files changed, 6 insertions(+), 8 deletions(-) -- 2.31.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH V1 1/2] mm/damon/dbgfs: Avoid target_ids display wrong pid value 2021-12-09 16:33 [PATCH V1 0/2] mm/damon: Do little changes Xin Hao @ 2021-12-09 16:33 ` Xin Hao 2021-12-09 16:33 ` [PATCH V1 2/2] mm/damon: Modify the display form of damon tracepoint Xin Hao 1 sibling, 0 replies; 4+ messages in thread From: Xin Hao @ 2021-12-09 16:33 UTC (permalink / raw) To: sj; +Cc: xhao, akpm, linux-mm, linux-kernel The return value of pid_vnr() is pid_t type which is defined by 'int', this may get a wrong pid value, if we set the return value type as 'unsigned long'. So there fix it. Signed-off-by: Xin Hao <xhao@linux.alibaba.com> --- mm/damon/dbgfs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c index 991b7fa0e858..97eebfb70ddf 100644 --- a/mm/damon/dbgfs.c +++ b/mm/damon/dbgfs.c @@ -281,17 +281,15 @@ static inline bool targetid_is_pid(const struct damon_ctx *ctx) static ssize_t sprint_target_ids(struct damon_ctx *ctx, char *buf, ssize_t len) { struct damon_target *t; - unsigned long id; - int written = 0; + int id, written = 0; int rc; damon_for_each_target(t, ctx) { - id = t->id; if (targetid_is_pid(ctx)) /* Show pid numbers to debugfs users */ - id = (unsigned long)pid_vnr((struct pid *)id); + id = (int)pid_vnr((struct pid *)t->id); - rc = scnprintf(&buf[written], len - written, "%lu ", id); + rc = scnprintf(&buf[written], len - written, "%d ", id); if (!rc) return -ENOMEM; written += rc; -- 2.31.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V1 2/2] mm/damon: Modify the display form of damon tracepoint 2021-12-09 16:33 [PATCH V1 0/2] mm/damon: Do little changes Xin Hao 2021-12-09 16:33 ` [PATCH V1 1/2] mm/damon/dbgfs: Avoid target_ids display wrong pid value Xin Hao @ 2021-12-09 16:33 ` Xin Hao 2021-12-09 16:46 ` SeongJae Park 1 sibling, 1 reply; 4+ messages in thread From: Xin Hao @ 2021-12-09 16:33 UTC (permalink / raw) To: sj; +Cc: xhao, akpm, linux-mm, linux-kernel 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); __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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V1 2/2] mm/damon: Modify the display form of damon tracepoint 2021-12-09 16:33 ` [PATCH V1 2/2] mm/damon: Modify the display form of damon tracepoint Xin Hao @ 2021-12-09 16:46 ` SeongJae Park 0 siblings, 0 replies; 4+ messages in thread From: SeongJae Park @ 2021-12-09 16:46 UTC (permalink / raw) To: Xin Hao; +Cc: sj, akpm, linux-mm, linux-kernel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-09 16:46 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-09 16:33 [PATCH V1 0/2] mm/damon: Do little changes Xin Hao 2021-12-09 16:33 ` [PATCH V1 1/2] mm/damon/dbgfs: Avoid target_ids display wrong pid value Xin Hao 2021-12-09 16:33 ` [PATCH V1 2/2] mm/damon: Modify the display form of damon tracepoint Xin Hao 2021-12-09 16:46 ` SeongJae Park
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox