2021-01-18 21:20:35

by Mark Brown

Subject: [PATCH] stacktrace: Move documentation for arch_stack_walk_reliable() to header

Currently arch_stack_wallk_reliable() is documented with an identical
comment in both x86 and S/390 implementations which is a bit redundant.
Move this to the header and convert to kerneldoc while we're at it.

Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: Miroslav Benes <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: Joe Lawrence <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Mark Brown <[email protected]>
---
arch/s390/kernel/stacktrace.c | 6 ------
arch/x86/kernel/stacktrace.c | 6 ------
include/linux/stacktrace.h | 19 +++++++++++++++++++
3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/arch/s390/kernel/stacktrace.c b/arch/s390/kernel/stacktrace.c
index 7f1266c24f6b..101477b3e263 100644
--- a/arch/s390/kernel/stacktrace.c
+++ b/arch/s390/kernel/stacktrace.c
@@ -24,12 +24,6 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
}
}

-/*
- * This function returns an error if it detects any unreliable features of the
- * stack. Otherwise it guarantees that the stack trace is reliable.
- *
- * If the task is not 'current', the caller *must* ensure the task is inactive.
- */
int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
void *cookie, struct task_struct *task)
{
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index 8627fda8d993..15b058eefc4e 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -29,12 +29,6 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
}
}

-/*
- * This function returns an error if it detects any unreliable features of the
- * stack. Otherwise it guarantees that the stack trace is reliable.
- *
- * If the task is not 'current', the caller *must* ensure the task is inactive.
- */
int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
void *cookie, struct task_struct *task)
{
diff --git a/include/linux/stacktrace.h b/include/linux/stacktrace.h
index 50e2df30b0aa..9edecb494e9e 100644
--- a/include/linux/stacktrace.h
+++ b/include/linux/stacktrace.h
@@ -52,8 +52,27 @@ typedef bool (*stack_trace_consume_fn)(void *cookie, unsigned long addr);
*/
void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
struct task_struct *task, struct pt_regs *regs);
+
+/**
+ * arch_stack_walk_reliable - Architecture specific function to walk the
+ * stack reliably
+ *
+ * @consume_entry: Callback which is invoked by the architecture code for
+ * each entry.
+ * @cookie: Caller supplied pointer which is handed back to
+ * @consume_entry
+ * @task: Pointer to a task struct, can be NULL
+ *
+ * This function returns an error if it detects any unreliable
+ * features of the stack. Otherwise it guarantees that the stack
+ * trace is reliable.
+ *
+ * If the task is not 'current', the caller *must* ensure the task is
+ * inactive and its stack is pinned.
+ */
int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry, void *cookie,
struct task_struct *task);
+
void arch_stack_walk_user(stack_trace_consume_fn consume_entry, void *cookie,
const struct pt_regs *regs);

--
2.20.1

2021-01-27 22:11:34

by Miroslav Benes

Subject: Re: [PATCH] stacktrace: Move documentation for arch_stack_walk_reliable() to header

On Mon, 18 Jan 2021, Mark Brown wrote:

> Currently arch_stack_wallk_reliable() is documented with an identical
> comment in both x86 and S/390 implementations which is a bit redundant.
> Move this to the header and convert to kerneldoc while we're at it.

Makes sense.

> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Heiko Carstens <[email protected]>
> Cc: Vasily Gorbik <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> Cc: Miroslav Benes <[email protected]>
> Cc: Petr Mladek <[email protected]>
> Cc: Joe Lawrence <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Mark Brown <[email protected]>

Reviewed-by: Miroslav Benes <[email protected]>

> diff --git a/arch/s390/kernel/stacktrace.c b/arch/s390/kernel/stacktrace.c
> index 7f1266c24f6b..101477b3e263 100644
> --- a/arch/s390/kernel/stacktrace.c
> +++ b/arch/s390/kernel/stacktrace.c
> @@ -24,12 +24,6 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
> }
> }
>
> -/*
> - * This function returns an error if it detects any unreliable features of the
> - * stack. Otherwise it guarantees that the stack trace is reliable.
> - *
> - * If the task is not 'current', the caller *must* ensure the task is inactive.
> - */
> int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
> void *cookie, struct task_struct *task)
> {

Just a note. arch/powerpc/kernel/stacktrace.c has the same for
__save_stack_trace_tsk_reliable(), but it would not be nice to pollute
include/linux/stacktrace.h with that in my opinion. It is an old
infrastructure after all.

Miroslav

2021-01-27 23:57:31

by Vasily Gorbik

Subject: Re: [PATCH] stacktrace: Move documentation for arch_stack_walk_reliable() to header

On Mon, Jan 18, 2021 at 09:10:21PM +0000, Mark Brown wrote:
> Currently arch_stack_wallk_reliable() is documented with an identical
> comment in both x86 and S/390 implementations which is a bit redundant.
> Move this to the header and convert to kerneldoc while we're at it.
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Heiko Carstens <[email protected]>
> Cc: Vasily Gorbik <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> Cc: Miroslav Benes <[email protected]>
> Cc: Petr Mladek <[email protected]>
> Cc: Joe Lawrence <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Mark Brown <[email protected]>
> ---
> arch/s390/kernel/stacktrace.c | 6 ------
> arch/x86/kernel/stacktrace.c | 6 ------
> include/linux/stacktrace.h | 19 +++++++++++++++++++
> 3 files changed, 19 insertions(+), 12 deletions(-)

Acked-by: Vasily Gorbik <[email protected]>