From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Andrew Morton <akpm@linux-foundation.org>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Florent Revest <revest@chromium.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
bpf <bpf@vger.kernel.org>, Sven Schnelle <svens@linux.ibm.com>,
Alexei Starovoitov <ast@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Alan Maguire <alan.maguire@oracle.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>, Guo Ren <guoren@kernel.org>
Subject: [PATCH 11/20] function_graph: Use a simple LRU for fgraph_array index number
Date: Fri, 24 May 2024 22:37:03 -0400 [thread overview]
Message-ID: <20240525023742.944236388@goodmis.org> (raw)
In-Reply-To: 20240525023652.903909489@goodmis.org
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Since the fgraph_array index is used for the bitmap on the shadow
stack, it may leave some entries after a function_graph instance is
removed. Thus if another instance reuses the fgraph_array index soon
after releasing it, the fgraph may confuse to call the newer callback
for the entries which are pushed by the older instance.
To avoid reusing the fgraph_array index soon after releasing, introduce
a simple LRU table for managing the index number. This will reduce the
possibility of this confusion.
Link: https://lore.kernel.org/linux-trace-kernel/171509103267.162236.6885097397289135378.stgit@devnote2
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 71 ++++++++++++++++++++++++++++++-------------
1 file changed, 50 insertions(+), 21 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 67fa7fbf6aac..8e029d5e94f6 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -125,10 +125,48 @@ enum {
DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph);
int ftrace_graph_active;
-static int fgraph_array_cnt;
-
static struct fgraph_ops *fgraph_array[FGRAPH_ARRAY_SIZE];
+/* LRU index table for fgraph_array */
+static int fgraph_lru_table[FGRAPH_ARRAY_SIZE];
+static int fgraph_lru_next;
+static int fgraph_lru_last;
+
+/* Initialize fgraph_lru_table with unused index */
+static void fgraph_lru_init(void)
+{
+ int i;
+
+ for (i = 0; i < FGRAPH_ARRAY_SIZE; i++)
+ fgraph_lru_table[i] = i;
+}
+
+/* Release the used index to the LRU table */
+static int fgraph_lru_release_index(int idx)
+{
+ if (idx < 0 || idx >= FGRAPH_ARRAY_SIZE ||
+ WARN_ON_ONCE(fgraph_lru_table[fgraph_lru_last] != -1))
+ return -1;
+
+ fgraph_lru_table[fgraph_lru_last] = idx;
+ fgraph_lru_last = (fgraph_lru_last + 1) % FGRAPH_ARRAY_SIZE;
+ return 0;
+}
+
+/* Allocate a new index from LRU table */
+static int fgraph_lru_alloc_index(void)
+{
+ int idx = fgraph_lru_table[fgraph_lru_next];
+
+ /* No id is available */
+ if (idx == -1)
+ return -1;
+
+ fgraph_lru_table[fgraph_lru_next] = -1;
+ fgraph_lru_next = (fgraph_lru_next + 1) % FGRAPH_ARRAY_SIZE;
+ return idx;
+}
+
/* Get the FRAME_OFFSET from the word from the @offset on ret_stack */
static inline int get_frame_offset(struct task_struct *t, int offset)
{
@@ -375,7 +413,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
if (offset < 0)
goto out;
- for (i = 0; i < fgraph_array_cnt; i++) {
+ for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
struct fgraph_ops *gops = fgraph_array[i];
if (gops == &fgraph_stub)
@@ -927,7 +965,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
{
int command = 0;
int ret = 0;
- int i;
+ int i = -1;
mutex_lock(&ftrace_lock);
@@ -943,21 +981,16 @@ int register_ftrace_graph(struct fgraph_ops *gops)
/* The array must always have real data on it */
for (i = 0; i < FGRAPH_ARRAY_SIZE; i++)
fgraph_array[i] = &fgraph_stub;
+ fgraph_lru_init();
}
- /* Look for an available spot */
- for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
- if (fgraph_array[i] == &fgraph_stub)
- break;
- }
- if (i >= FGRAPH_ARRAY_SIZE) {
+ i = fgraph_lru_alloc_index();
+ if (i < 0 || WARN_ON_ONCE(fgraph_array[i] != &fgraph_stub)) {
ret = -ENOSPC;
goto out;
}
fgraph_array[i] = gops;
- if (i + 1 > fgraph_array_cnt)
- fgraph_array_cnt = i + 1;
gops->idx = i;
ftrace_graph_active++;
@@ -981,6 +1014,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
if (ret) {
fgraph_array[i] = &fgraph_stub;
ftrace_graph_active--;
+ fgraph_lru_release_index(i);
}
out:
mutex_unlock(&ftrace_lock);
@@ -990,25 +1024,20 @@ int register_ftrace_graph(struct fgraph_ops *gops)
void unregister_ftrace_graph(struct fgraph_ops *gops)
{
int command = 0;
- int i;
mutex_lock(&ftrace_lock);
if (unlikely(!ftrace_graph_active))
goto out;
- if (unlikely(gops->idx < 0 || gops->idx >= fgraph_array_cnt))
+ if (unlikely(gops->idx < 0 || gops->idx >= FGRAPH_ARRAY_SIZE ||
+ fgraph_array[gops->idx] != gops))
goto out;
- WARN_ON_ONCE(fgraph_array[gops->idx] != gops);
+ if (fgraph_lru_release_index(gops->idx) < 0)
+ goto out;
fgraph_array[gops->idx] = &fgraph_stub;
- if (gops->idx + 1 == fgraph_array_cnt) {
- i = gops->idx;
- while (i >= 0 && fgraph_array[i] == &fgraph_stub)
- i--;
- fgraph_array_cnt = i + 1;
- }
ftrace_graph_active--;
--
2.43.0
next prev parent reply other threads:[~2024-05-25 2:36 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-25 2:36 [PATCH 00/20] function_graph: Allow multiple users for function graph tracing Steven Rostedt
2024-05-25 2:36 ` [PATCH 01/20] function_graph: Convert ret_stack to a series of longs Steven Rostedt
2024-05-25 2:36 ` [PATCH 02/20] fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long Steven Rostedt
2024-05-25 2:36 ` [PATCH 03/20] function_graph: Add an array structure that will allow multiple callbacks Steven Rostedt
2024-05-25 2:36 ` [PATCH 04/20] function_graph: Allow multiple users to attach to function graph Steven Rostedt
2024-05-27 0:34 ` Masami Hiramatsu
2024-05-27 1:17 ` Steven Rostedt
2024-05-25 2:36 ` [PATCH 05/20] function_graph: Handle tail calls for stack unwinding Steven Rostedt
2024-05-25 2:36 ` [PATCH 06/20] function_graph: Remove logic around ftrace_graph_entry and return Steven Rostedt
2024-05-25 2:36 ` [PATCH 07/20] ftrace/function_graph: Pass fgraph_ops to function graph callbacks Steven Rostedt
2024-05-25 2:37 ` [PATCH 08/20] ftrace: Allow function_graph tracer to be enabled in instances Steven Rostedt
2024-05-25 2:37 ` [PATCH 09/20] ftrace: Allow ftrace startup flags to exist without dynamic ftrace Steven Rostedt
2024-05-25 2:37 ` [PATCH 10/20] function_graph: Have the instances use their own ftrace_ops for filtering Steven Rostedt
2024-05-31 2:30 ` Steven Rostedt
2024-05-31 3:12 ` Masami Hiramatsu
2024-05-31 6:03 ` Steven Rostedt
2024-05-31 14:50 ` Masami Hiramatsu
2024-05-31 22:49 ` Steven Rostedt
2024-06-01 19:19 ` Steven Rostedt
2024-06-02 2:40 ` Masami Hiramatsu
2024-05-25 2:37 ` Steven Rostedt [this message]
2024-05-25 2:37 ` [PATCH 12/20] function_graph: Add "task variables" per task for fgraph_ops Steven Rostedt
2024-05-25 2:37 ` [PATCH 13/20] function_graph: Move set_graph_function tests to shadow stack global var Steven Rostedt
2024-05-25 2:37 ` [PATCH 14/20] function_graph: Move graph depth stored data " Steven Rostedt
2024-05-25 2:37 ` [PATCH 15/20] function_graph: Move graph notrace bit " Steven Rostedt
2024-05-25 2:37 ` [PATCH 16/20] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data() Steven Rostedt
2024-05-25 2:37 ` [PATCH 17/20] function_graph: Add selftest for passing local variables Steven Rostedt
2024-05-25 2:37 ` [PATCH 18/20] ftrace: Add multiple fgraph storage selftest Steven Rostedt
2024-05-25 2:37 ` [PATCH 19/20] function_graph: Use for_each_set_bit() in __ftrace_return_to_handler() Steven Rostedt
2024-05-26 23:58 ` Masami Hiramatsu
2024-05-27 0:04 ` Masami Hiramatsu
2024-05-27 0:32 ` Steven Rostedt
2024-05-25 2:37 ` [PATCH 20/20] function_graph: Use bitmask to loop on fgraph entry Steven Rostedt
2024-05-27 0:09 ` Masami Hiramatsu
2024-05-27 0:33 ` Steven Rostedt
2024-05-27 0:37 ` [PATCH 00/20] function_graph: Allow multiple users for function graph tracing Masami Hiramatsu
2024-05-27 1:18 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240525023742.944236388@goodmis.org \
--to=rostedt@goodmis.org \
--cc=acme@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=alan.maguire@oracle.com \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=guoren@kernel.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=martin.lau@linux.dev \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=peterz@infradead.org \
--cc=revest@chromium.org \
--cc=svens@linux.ibm.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.