2023-10-13 19:05:22

by Audra Mitchell

[permalink] [raw]
Subject: [PATCH 0/5] Fix page_owner's use of free timestamps

While page ower output is used to investigate memory utilization, typically
the allocation pathway, the introduction of timestamps to the page owner
records caused each record to become unique due to the granularity of the
nanosecond timestamp (for example):

Page allocated via order 0 ... ts 5206196026 ns, free_ts 5187156703 ns
Page allocated via order 0 ... ts 5206198540 ns, free_ts 5187162702 ns

Furthermore, the page_owner output only dumps the currently allocated
records, so having the free timestamps is nonsensical for the typical use
case.

In addition, the introduction of timestamps was not properly handled in
the page_owner_sort tool causing most use cases to be broken. This series
is meant to remove the free timestamps from the page_owner output and
fix the page_owner_sort tool so proper collation can occur.

Audra Mitchell (5):
mm/page_owner: Remove free_ts from page_owner output
tools/mm: Remove references to free_ts from page_owner_sort
tools/mm: Filter out timestamps for correct collation
tools/mm: Fix the default case for page_owner_sort
tools/mm: Update the usage output to be more organized

mm/page_owner.c | 4 +-
tools/mm/page_owner_sort.c | 212 +++++++++++++++++--------------------
2 files changed, 100 insertions(+), 116 deletions(-)

--
2.41.0


2023-10-13 19:05:36

by Audra Mitchell

[permalink] [raw]
Subject: [PATCH 4/5] tools/mm: Fix the default case for page_owner_sort

With the additional commands and timestamps added to the tool, the default
case (-t) has been broken. Now that the allocation timestamps are saved
outside of the txt field, allow us to properly sort the data by number of
times the record has been seen. Furthermore prevent the misuse of the
commandline arguments so only one compare option can be used.

Signed-off-by: Audra Mitchell <[email protected]>
---
tools/mm/page_owner_sort.c | 61 +++++++++++++++++++++++++++++++++-----
1 file changed, 53 insertions(+), 8 deletions(-)

diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index 7ddabcb3a073..5a260096ebaa 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -66,6 +66,16 @@ enum SORT_ORDER {
SORT_ASC = 1,
SORT_DESC = -1,
};
+enum COMP_FLAG {
+ COMP_NO_FLAG = 0,
+ COMP_ALLOC = 1<<0,
+ COMP_PAGE_NUM = 1<<1,
+ COMP_PID = 1<<2,
+ COMP_STACK = 1<<3,
+ COMP_NUM = 1<<4,
+ COMP_TGID = 1<<5,
+ COMP_COMM = 1<<6
+};
struct filter_condition {
pid_t *pids;
pid_t *tgids;
@@ -644,7 +654,7 @@ int main(int argc, char **argv)
{
FILE *fin, *fout;
char *buf, *ext_buf;
- int i, count;
+ int i, count, compare_flag;
struct stat st;
int opt;
struct option longopts[] = {
@@ -656,31 +666,33 @@ int main(int argc, char **argv)
{ 0, 0, 0, 0},
};

+ compare_flag = COMP_NO_FLAG;
+
while ((opt = getopt_long(argc, argv, "admnpstP", longopts, NULL)) != -1)
switch (opt) {
case 'a':
- set_single_cmp(compare_ts, SORT_ASC);
+ compare_flag |= COMP_ALLOC;
break;
case 'd':
debug_on = true;
break;
case 'm':
- set_single_cmp(compare_page_num, SORT_DESC);
+ compare_flag |= COMP_PAGE_NUM;
break;
case 'p':
- set_single_cmp(compare_pid, SORT_ASC);
+ compare_flag |= COMP_PID;
break;
case 's':
- set_single_cmp(compare_stacktrace, SORT_ASC);
+ compare_flag |= COMP_STACK;
break;
case 't':
- set_single_cmp(compare_num, SORT_DESC);
+ compare_flag |= COMP_NUM;
break;
case 'P':
- set_single_cmp(compare_tgid, SORT_ASC);
+ compare_flag |= COMP_TGID;
break;
case 'n':
- set_single_cmp(compare_comm, SORT_ASC);
+ compare_flag |= COMP_COMM;
break;
case 1:
filter = filter | FILTER_PID;
@@ -728,6 +740,39 @@ int main(int argc, char **argv)
exit(1);
}

+ /* Only one compare option is allowed, yet we also want handle the
+ * default case were no option is provided, but we still want to
+ * match the behavior of the -t option (compare by number of times
+ * a record is seen
+ */
+ switch (compare_flag) {
+ case COMP_ALLOC:
+ set_single_cmp(compare_ts, SORT_ASC);
+ break;
+ case COMP_PAGE_NUM:
+ set_single_cmp(compare_page_num, SORT_DESC);
+ break;
+ case COMP_PID:
+ set_single_cmp(compare_pid, SORT_ASC);
+ break;
+ case COMP_STACK:
+ set_single_cmp(compare_stacktrace, SORT_ASC);
+ break;
+ case COMP_NO_FLAG:
+ case COMP_NUM:
+ set_single_cmp(compare_num, SORT_DESC);
+ break;
+ case COMP_TGID:
+ set_single_cmp(compare_tgid, SORT_ASC);
+ break;
+ case COMP_COMM:
+ set_single_cmp(compare_comm, SORT_ASC);
+ break;
+ default:
+ usage();
+ exit(1);
+ }
+
fin = fopen(argv[optind], "r");
fout = fopen(argv[optind + 1], "w");
if (!fin || !fout) {
--
2.41.0

2023-10-16 11:56:33

by Rafael Aquini

[permalink] [raw]
Subject: Re: [PATCH 0/5] Fix page_owner's use of free timestamps

On Fri, Oct 13, 2023 at 03:03:44PM -0400, Audra Mitchell wrote:
> While page ower output is used to investigate memory utilization, typically
> the allocation pathway, the introduction of timestamps to the page owner
> records caused each record to become unique due to the granularity of the
> nanosecond timestamp (for example):
>
> Page allocated via order 0 ... ts 5206196026 ns, free_ts 5187156703 ns
> Page allocated via order 0 ... ts 5206198540 ns, free_ts 5187162702 ns
>
> Furthermore, the page_owner output only dumps the currently allocated
> records, so having the free timestamps is nonsensical for the typical use
> case.
>
> In addition, the introduction of timestamps was not properly handled in
> the page_owner_sort tool causing most use cases to be broken. This series
> is meant to remove the free timestamps from the page_owner output and
> fix the page_owner_sort tool so proper collation can occur.
>
> Audra Mitchell (5):
> mm/page_owner: Remove free_ts from page_owner output
> tools/mm: Remove references to free_ts from page_owner_sort
> tools/mm: Filter out timestamps for correct collation
> tools/mm: Fix the default case for page_owner_sort
> tools/mm: Update the usage output to be more organized
>
> mm/page_owner.c | 4 +-
> tools/mm/page_owner_sort.c | 212 +++++++++++++++++--------------------
> 2 files changed, 100 insertions(+), 116 deletions(-)
>
> --
> 2.41.0
>

Thank you, Audra.

Acked-by: Rafael Aquini <[email protected]>

2023-10-17 08:13:19

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH 4/5] tools/mm: Fix the default case for page_owner_sort

On 10/13/23 21:03, Audra Mitchell wrote:
> With the additional commands and timestamps added to the tool, the default
> case (-t) has been broken. Now that the allocation timestamps are saved
> outside of the txt field, allow us to properly sort the data by number of
> times the record has been seen. Furthermore prevent the misuse of the
> commandline arguments so only one compare option can be used.
>
> Signed-off-by: Audra Mitchell <[email protected]>

Acked-by: Vlastimil Babka <[email protected]>