2018-11-22 13:35:31

by Steven Rostedt

[permalink] [raw]
Subject: [RFC][PATCH 09/14] function_graph: Move ftrace_graph_get_addr() to fgraph.c

From: "Steven Rostedt (VMware)" <[email protected]>

Move the function function_graph_get_addr() to fgraph.c, as the management
of the curr_ret_stack is going to change, and all the accesses to ret_stack
needs to be done in fgraph.c.

Signed-off-by: Steven Rostedt (VMware) <[email protected]>
---
kernel/trace/fgraph.c | 55 ++++++++++++++++++++++++++++
kernel/trace/trace_functions_graph.c | 55 ----------------------------
2 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index f3a89ecac671..c7d612897e33 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -233,6 +233,61 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
return ret;
}

+/**
+ * ftrace_graph_ret_addr - convert a potentially modified stack return address
+ * to its original value
+ *
+ * This function can be called by stack unwinding code to convert a found stack
+ * return address ('ret') to its original value, in case the function graph
+ * tracer has modified it to be 'return_to_handler'. If the address hasn't
+ * been modified, the unchanged value of 'ret' is returned.
+ *
+ * 'idx' is a state variable which should be initialized by the caller to zero
+ * before the first call.
+ *
+ * 'retp' is a pointer to the return address on the stack. It's ignored if
+ * the arch doesn't have HAVE_FUNCTION_GRAPH_RET_ADDR_PTR defined.
+ */
+#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
+unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
+ unsigned long ret, unsigned long *retp)
+{
+ int index = task->curr_ret_stack;
+ int i;
+
+ if (ret != (unsigned long)return_to_handler)
+ return ret;
+
+ if (index < 0)
+ return ret;
+
+ for (i = 0; i <= index; i++)
+ if (task->ret_stack[i].retp == retp)
+ return task->ret_stack[i].ret;
+
+ return ret;
+}
+#else /* !HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
+unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
+ unsigned long ret, unsigned long *retp)
+{
+ int task_idx;
+
+ if (ret != (unsigned long)return_to_handler)
+ return ret;
+
+ task_idx = task->curr_ret_stack;
+
+ if (!task->ret_stack || task_idx < *idx)
+ return ret;
+
+ task_idx -= *idx;
+ (*idx)++;
+
+ return task->ret_stack[task_idx].ret;
+}
+#endif /* HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
+
static struct ftrace_ops graph_ops = {
.func = ftrace_stub,
.flags = FTRACE_OPS_FL_RECURSION_SAFE |
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 7c7fd13d2373..0f9cbc30645d 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -90,61 +90,6 @@ static void
print_graph_duration(struct trace_array *tr, unsigned long long duration,
struct trace_seq *s, u32 flags);

-/**
- * ftrace_graph_ret_addr - convert a potentially modified stack return address
- * to its original value
- *
- * This function can be called by stack unwinding code to convert a found stack
- * return address ('ret') to its original value, in case the function graph
- * tracer has modified it to be 'return_to_handler'. If the address hasn't
- * been modified, the unchanged value of 'ret' is returned.
- *
- * 'idx' is a state variable which should be initialized by the caller to zero
- * before the first call.
- *
- * 'retp' is a pointer to the return address on the stack. It's ignored if
- * the arch doesn't have HAVE_FUNCTION_GRAPH_RET_ADDR_PTR defined.
- */
-#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
-unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
- unsigned long ret, unsigned long *retp)
-{
- int index = task->curr_ret_stack;
- int i;
-
- if (ret != (unsigned long)return_to_handler)
- return ret;
-
- if (index < 0)
- return ret;
-
- for (i = 0; i <= index; i++)
- if (task->ret_stack[i].retp == retp)
- return task->ret_stack[i].ret;
-
- return ret;
-}
-#else /* !HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
-unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
- unsigned long ret, unsigned long *retp)
-{
- int task_idx;
-
- if (ret != (unsigned long)return_to_handler)
- return ret;
-
- task_idx = task->curr_ret_stack;
-
- if (!task->ret_stack || task_idx < *idx)
- return ret;
-
- task_idx -= *idx;
- (*idx)++;
-
- return task->ret_stack[task_idx].ret;
-}
-#endif /* HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
-
int __trace_graph_entry(struct trace_array *tr,
struct ftrace_graph_ent *trace,
unsigned long flags,
--
2.19.1




2018-11-24 07:56:40

by Joel Fernandes

[permalink] [raw]
Subject: Re: [RFC][PATCH 09/14] function_graph: Move ftrace_graph_get_addr() to fgraph.c

On Wed, Nov 21, 2018 at 08:27:17PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <[email protected]>
>
> Move the function function_graph_get_addr() to fgraph.c, as the management
> of the curr_ret_stack is going to change, and all the accesses to ret_stack
> needs to be done in fgraph.c.

s/ftrace_graph_get_addr/ftrace_graph_ret_addr/

thanks,

- Joel

>
> Signed-off-by: Steven Rostedt (VMware) <[email protected]>
> ---
> kernel/trace/fgraph.c | 55 ++++++++++++++++++++++++++++
> kernel/trace/trace_functions_graph.c | 55 ----------------------------
> 2 files changed, 55 insertions(+), 55 deletions(-)
>
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index f3a89ecac671..c7d612897e33 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -233,6 +233,61 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
> return ret;
> }
>
> +/**
> + * ftrace_graph_ret_addr - convert a potentially modified stack return address
> + * to its original value
> + *
> + * This function can be called by stack unwinding code to convert a found stack
> + * return address ('ret') to its original value, in case the function graph
> + * tracer has modified it to be 'return_to_handler'. If the address hasn't
> + * been modified, the unchanged value of 'ret' is returned.
> + *
> + * 'idx' is a state variable which should be initialized by the caller to zero
> + * before the first call.
> + *
> + * 'retp' is a pointer to the return address on the stack. It's ignored if
> + * the arch doesn't have HAVE_FUNCTION_GRAPH_RET_ADDR_PTR defined.
> + */
> +#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
> +unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
> + unsigned long ret, unsigned long *retp)
> +{
> + int index = task->curr_ret_stack;
> + int i;
> +
> + if (ret != (unsigned long)return_to_handler)
> + return ret;
> +
> + if (index < 0)
> + return ret;
> +
> + for (i = 0; i <= index; i++)
> + if (task->ret_stack[i].retp == retp)
> + return task->ret_stack[i].ret;
> +
> + return ret;
> +}
> +#else /* !HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
> +unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
> + unsigned long ret, unsigned long *retp)
> +{
> + int task_idx;
> +
> + if (ret != (unsigned long)return_to_handler)
> + return ret;
> +
> + task_idx = task->curr_ret_stack;
> +
> + if (!task->ret_stack || task_idx < *idx)
> + return ret;
> +
> + task_idx -= *idx;
> + (*idx)++;
> +
> + return task->ret_stack[task_idx].ret;
> +}
> +#endif /* HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
> +
> static struct ftrace_ops graph_ops = {
> .func = ftrace_stub,
> .flags = FTRACE_OPS_FL_RECURSION_SAFE |
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index 7c7fd13d2373..0f9cbc30645d 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -90,61 +90,6 @@ static void
> print_graph_duration(struct trace_array *tr, unsigned long long duration,
> struct trace_seq *s, u32 flags);
>
> -/**
> - * ftrace_graph_ret_addr - convert a potentially modified stack return address
> - * to its original value
> - *
> - * This function can be called by stack unwinding code to convert a found stack
> - * return address ('ret') to its original value, in case the function graph
> - * tracer has modified it to be 'return_to_handler'. If the address hasn't
> - * been modified, the unchanged value of 'ret' is returned.
> - *
> - * 'idx' is a state variable which should be initialized by the caller to zero
> - * before the first call.
> - *
> - * 'retp' is a pointer to the return address on the stack. It's ignored if
> - * the arch doesn't have HAVE_FUNCTION_GRAPH_RET_ADDR_PTR defined.
> - */
> -#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
> -unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
> - unsigned long ret, unsigned long *retp)
> -{
> - int index = task->curr_ret_stack;
> - int i;
> -
> - if (ret != (unsigned long)return_to_handler)
> - return ret;
> -
> - if (index < 0)
> - return ret;
> -
> - for (i = 0; i <= index; i++)
> - if (task->ret_stack[i].retp == retp)
> - return task->ret_stack[i].ret;
> -
> - return ret;
> -}
> -#else /* !HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
> -unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
> - unsigned long ret, unsigned long *retp)
> -{
> - int task_idx;
> -
> - if (ret != (unsigned long)return_to_handler)
> - return ret;
> -
> - task_idx = task->curr_ret_stack;
> -
> - if (!task->ret_stack || task_idx < *idx)
> - return ret;
> -
> - task_idx -= *idx;
> - (*idx)++;
> -
> - return task->ret_stack[task_idx].ret;
> -}
> -#endif /* HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
> -
> int __trace_graph_entry(struct trace_array *tr,
> struct ftrace_graph_ent *trace,
> unsigned long flags,
> --
> 2.19.1
>
>

2018-11-24 08:49:25

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC][PATCH 09/14] function_graph: Move ftrace_graph_get_addr() to fgraph.c

On Thu, 22 Nov 2018 19:13:38 -0800
Joel Fernandes <[email protected]> wrote:

> On Wed, Nov 21, 2018 at 08:27:17PM -0500, Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <[email protected]>
> >
> > Move the function function_graph_get_addr() to fgraph.c, as the management
> > of the curr_ret_stack is going to change, and all the accesses to ret_stack
> > needs to be done in fgraph.c.
>
> s/ftrace_graph_get_addr/ftrace_graph_ret_addr/
>

Good catch. I wrote the change logs a bit after writing the patch. I
had "get" on my mind.

-- Steve