From c5f0903d6f47d6f04ce0710f9d089f0c51b374bf Mon Sep 17 00:00:00 2001 From: Daon Park Date: Tue, 29 Oct 2024 09:53:31 +0000 Subject: [PATCH 1/6] utils: Identify graph entry nodes to skip Identify if the current graph entry must be skipped druing `dump`, which is done during the add_graph_entry() procedure. Basic idea is to find recursive calls, but the first recursive call cannot be skipped. For example, if the call trace shows: main - fib - fib - fib then the first (fib - fib) must be present but the second must be skipped, making the graph look like: main - fib - fib To separate recursive calls from the first recursive call, determine if the node's name is the same with curr and additionally check if curr->parent's name is the same as well. Signed-off-by: Daon Park --- utils/graph.c | 6 ++++++ utils/graph.h | 1 + 2 files changed, 7 insertions(+) diff --git a/utils/graph.c b/utils/graph.c index 115eaed6b..ba9557367 100644 --- a/utils/graph.c +++ b/utils/graph.c @@ -63,6 +63,7 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod struct uftrace_graph_node *curr = tg->node; struct uftrace_fstack *fstack; static uint32_t next_id = 1; + static bool skip = false; if (tg->lost) return 1; /* ignore kernel functions after LOST */ @@ -77,6 +78,10 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod if (curr == NULL || fstack == NULL) return -1; + if (name && curr && curr->parent) { + skip = !strcmp(name, curr->name) && !strcmp(name, curr->parent->name); + } + list_for_each_entry(node, &curr->head, list) { if (name && !strcmp(name, node->name)) break; @@ -91,6 +96,7 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod node->id = next_id++; node->addr = fstack->addr; node->name = xstrdup(name ?: "none"); + node->skip = skip; INIT_LIST_HEAD(&node->head); node->parent = curr; diff --git a/utils/graph.h b/utils/graph.h index 598f36b3d..ef6ef46ba 100644 --- a/utils/graph.h +++ b/utils/graph.h @@ -17,6 +17,7 @@ struct uftrace_graph_node { uint64_t time; uint64_t child_time; uint32_t id; + bool skip; struct list_head head; struct list_head list; struct uftrace_graph_node *parent; From 8d47c7fc4b806aa255b88ec0bd6cbf8d8952e2bd Mon Sep 17 00:00:00 2001 From: Daon Park Date: Tue, 29 Oct 2024 09:55:26 +0000 Subject: [PATCH 2/6] utils: Transfer skipped node's nr_calls If a node is skipped due to being a recursive call, then it's nr_calls must be correctly appended to the first recursive call. During the `add_graph_entry()` procedure, it looks for the source of the first recursive call and saves it to recursive_src. Next, if the current graph entry node is skipped, then the nr_calls is added to the add_calls_tgt. Signed-off-by: Daon Park --- utils/graph.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/utils/graph.c b/utils/graph.c index ba9557367..c9afe497a 100644 --- a/utils/graph.c +++ b/utils/graph.c @@ -61,6 +61,8 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod { struct uftrace_graph_node *node = NULL; struct uftrace_graph_node *curr = tg->node; + struct uftrace_graph_node *recursive_src = NULL; + struct uftrace_graph_node *add_calls_tgt = NULL; struct uftrace_fstack *fstack; static uint32_t next_id = 1; static bool skip = false; @@ -78,10 +80,18 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod if (curr == NULL || fstack == NULL) return -1; - if (name && curr && curr->parent) { - skip = !strcmp(name, curr->name) && !strcmp(name, curr->parent->name); + if (name) { + if (curr && curr->parent) { + skip = !strcmp(name, curr->name) && !strcmp(name, curr->parent->name); + } + + while (curr && !strcmp(name, curr->name)) { + recursive_src = curr; + curr = curr->parent; + } } + curr = tg->node; list_for_each_entry(node, &curr->head, list) { if (name && !strcmp(name, node->name)) break; @@ -134,6 +144,13 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod out: node->nr_calls++; + if (skip) { + list_for_each_entry(add_calls_tgt, &recursive_src->head, list) { + if (name && !strcmp(name, add_calls_tgt->name)) + break; + } + add_calls_tgt->nr_calls++; + } tg->node = node; if (entry_cb) From 7e089cd9b1a86d70040923b33f5b54a5510274ac Mon Sep 17 00:00:00 2001 From: Daon Park Date: Tue, 29 Oct 2024 09:59:11 +0000 Subject: [PATCH 3/6] dump: Skip nodes during graphviz dump If a node is marked as True in the skip variable, the dump file will not record that node. Signed-off-by: Daon Park --- cmds/dump.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmds/dump.c b/cmds/dump.c index 74719f11e..89733d400 100644 --- a/cmds/dump.c +++ b/cmds/dump.c @@ -1258,8 +1258,9 @@ static void print_graph_to_graphviz(struct uftrace_graph_node *node, struct uftr { struct uftrace_graph_node *child; unsigned long n_calls = node->nr_calls; + bool skip = node->skip; - if (n_calls) { + if (n_calls && !skip) { struct uftrace_graph_node *parent = node->parent; pr_out(" "); From 4ebd361bbf78b2afddec20b2c50821bd3ecde983 Mon Sep 17 00:00:00 2001 From: Daon Park Date: Tue, 29 Oct 2024 10:53:56 +0000 Subject: [PATCH 4/6] utils: Check NULL during recursive source search During a search for the source of the recursive call, it must check for NULL pointers. This passes the unittest 51 graph_basic. Signed-off-by: Daon Park --- utils/graph.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/utils/graph.c b/utils/graph.c index c9afe497a..763b75978 100644 --- a/utils/graph.c +++ b/utils/graph.c @@ -61,6 +61,7 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod { struct uftrace_graph_node *node = NULL; struct uftrace_graph_node *curr = tg->node; + struct uftrace_graph_node *search_curr = NULL; struct uftrace_graph_node *recursive_src = NULL; struct uftrace_graph_node *add_calls_tgt = NULL; struct uftrace_fstack *fstack; @@ -85,13 +86,13 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod skip = !strcmp(name, curr->name) && !strcmp(name, curr->parent->name); } - while (curr && !strcmp(name, curr->name)) { - recursive_src = curr; - curr = curr->parent; + search_curr = curr; + while (search_curr && search_curr->name && !strcmp(name, search_curr->name)) { + recursive_src = search_curr; + search_curr = search_curr->parent; } } - curr = tg->node; list_for_each_entry(node, &curr->head, list) { if (name && !strcmp(name, node->name)) break; From 905bfa553b26097d817d0cf7eb95b45df911ff33 Mon Sep 17 00:00:00 2001 From: Daon Park Date: Tue, 29 Oct 2024 10:58:29 +0000 Subject: [PATCH 5/6] utils: Check NULL during nr_calls node search During a search for the destination node for the nr_calls, it must check for NULL pointers. This passes the unittest 52 graph_command. Signed-off-by: Daon Park --- utils/graph.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/graph.c b/utils/graph.c index 763b75978..f3290ad10 100644 --- a/utils/graph.c +++ b/utils/graph.c @@ -145,7 +145,7 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod out: node->nr_calls++; - if (skip) { + if (skip && recursive_src) { list_for_each_entry(add_calls_tgt, &recursive_src->head, list) { if (name && !strcmp(name, add_calls_tgt->name)) break; From fd235ae3b15aff315524706b9411d4f346ab32e2 Mon Sep 17 00:00:00 2001 From: Daon Park Date: Tue, 29 Oct 2024 11:12:48 +0000 Subject: [PATCH 6/6] utils: Check NULL during skip node determination During the determination of a node skip, it must check for NULL pointers in the name. This passes the unittest 105 tui_command. Signed-off-by: Daon Park --- utils/graph.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/graph.c b/utils/graph.c index f3290ad10..1ec40427e 100644 --- a/utils/graph.c +++ b/utils/graph.c @@ -82,7 +82,7 @@ static int add_graph_entry(struct uftrace_task_graph *tg, char *name, size_t nod return -1; if (name) { - if (curr && curr->parent) { + if (curr && curr->name && curr->parent && curr->parent->name) { skip = !strcmp(name, curr->name) && !strcmp(name, curr->parent->name); }