Re: [RFC patch 40/41] stacktrace: Remove obsolete functions
On Wed, Apr 10, 2019 at 10:33:20PM -0500, Josh Poimboeuf wrote: > > -#ifdef CONFIG_USER_STACKTRACE_SUPPORT > > +extern void save_stack_trace(struct stack_trace *trace); > > +extern void save_stack_trace_regs(struct pt_regs *regs, > > + struct stack_trace *trace); > > +extern void save_stack_trace_tsk(struct task_struct *tsk, > > + struct stack_trace *trace); > > +extern int save_stack_trace_tsk_reliable(struct task_struct *tsk, > > +struct stack_trace *trace); > > save_stack_trace_tsk_reliable() is still in use by generic livepatch > code. kernel/trace/trace_stack.c and include/linux/ftrace.h also still use struct stack_trace. -- Josh
Re: [RFC patch 40/41] stacktrace: Remove obsolete functions
On Wed, Apr 10, 2019 at 10:33:20PM -0500, Josh Poimboeuf wrote:
> On Wed, Apr 10, 2019 at 12:28:34PM +0200, Thomas Gleixner wrote:
> > +struct stack_trace {
> > + unsigned int nr_entries, max_entries;
> > + unsigned long *entries;
> > + int skip; /* input argument: How many entries to skip */
> > +};
>
> I was a bit surprised to see struct stack_trace still standing at the
> end of the patch set, but I guess 41 patches is enough :-) Do we want
> to eventually remove the struct altogether?
>
> I was also hoping to see the fragile "skipnr" go away in favor of
> something less dependent on compiler optimizations, but I'm not sure how
> feasible that would be.
It will die, but that only takes another nr_arch+1 patches.
Re: [RFC patch 40/41] stacktrace: Remove obsolete functions
On Wed, Apr 10, 2019 at 12:28:34PM +0200, Thomas Gleixner wrote:
> No more users of the struct stack_trace based interfaces. Remove them.
>
> Remove the macro stubs for !CONFIG_STACKTRACE as well as they are pointless
> because the storage on the call sites is conditional on CONFIG_STACKTRACE
> already. No point to be 'smart'.
>
> Signed-off-by: Thomas Gleixner
> ---
> include/linux/stacktrace.h | 46
> +++--
> kernel/stacktrace.c| 14 -
> 2 files changed, 16 insertions(+), 44 deletions(-)
>
> --- a/include/linux/stacktrace.h
> +++ b/include/linux/stacktrace.h
> @@ -8,23 +8,6 @@ struct task_struct;
> struct pt_regs;
>
> #ifdef CONFIG_STACKTRACE
> -struct stack_trace {
> - unsigned int nr_entries, max_entries;
> - unsigned long *entries;
> - int skip; /* input argument: How many entries to skip */
> -};
> -
> -extern void save_stack_trace(struct stack_trace *trace);
> -extern void save_stack_trace_regs(struct pt_regs *regs,
> - struct stack_trace *trace);
> -extern void save_stack_trace_tsk(struct task_struct *tsk,
> - struct stack_trace *trace);
> -extern int save_stack_trace_tsk_reliable(struct task_struct *tsk,
> - struct stack_trace *trace);
> -
> -extern void print_stack_trace(struct stack_trace *trace, int spaces);
> -extern int snprint_stack_trace(char *buf, size_t size,
> - struct stack_trace *trace, int spaces);
>
> extern void stack_trace_print(unsigned long *trace, unsigned int nr_entries,
> int spaces);
> @@ -43,20 +26,23 @@ extern unsigned int stack_trace_save_reg
> extern unsigned int stack_trace_save_user(unsigned long *store,
> unsigned int size,
> unsigned int skipnr);
> +/*
> + * The below is for stack trace internals and architecture
> + * implementations. Do not use in generic code.
> + */
> +struct stack_trace {
> + unsigned int nr_entries, max_entries;
> + unsigned long *entries;
> + int skip; /* input argument: How many entries to skip */
> +};
I was a bit surprised to see struct stack_trace still standing at the
end of the patch set, but I guess 41 patches is enough :-) Do we want
to eventually remove the struct altogether?
I was also hoping to see the fragile "skipnr" go away in favor of
something less dependent on compiler optimizations, but I'm not sure how
feasible that would be.
Regardless, these are very nice cleanups, nice work.
> -#ifdef CONFIG_USER_STACKTRACE_SUPPORT
> +extern void save_stack_trace(struct stack_trace *trace);
> +extern void save_stack_trace_regs(struct pt_regs *regs,
> + struct stack_trace *trace);
> +extern void save_stack_trace_tsk(struct task_struct *tsk,
> + struct stack_trace *trace);
> +extern int save_stack_trace_tsk_reliable(struct task_struct *tsk,
> + struct stack_trace *trace);
save_stack_trace_tsk_reliable() is still in use by generic livepatch
code.
Also I wonder if it would make sense to rename these to
__save_stack_trace_*() or arch_save_stack_trace_*() to help discourage
them from being used by generic code.
--
Josh
[RFC patch 40/41] stacktrace: Remove obsolete functions
No more users of the struct stack_trace based interfaces. Remove them.
Remove the macro stubs for !CONFIG_STACKTRACE as well as they are pointless
because the storage on the call sites is conditional on CONFIG_STACKTRACE
already. No point to be 'smart'.
Signed-off-by: Thomas Gleixner
---
include/linux/stacktrace.h | 46 +++--
kernel/stacktrace.c| 14 -
2 files changed, 16 insertions(+), 44 deletions(-)
--- a/include/linux/stacktrace.h
+++ b/include/linux/stacktrace.h
@@ -8,23 +8,6 @@ struct task_struct;
struct pt_regs;
#ifdef CONFIG_STACKTRACE
-struct stack_trace {
- unsigned int nr_entries, max_entries;
- unsigned long *entries;
- int skip; /* input argument: How many entries to skip */
-};
-
-extern void save_stack_trace(struct stack_trace *trace);
-extern void save_stack_trace_regs(struct pt_regs *regs,
- struct stack_trace *trace);
-extern void save_stack_trace_tsk(struct task_struct *tsk,
- struct stack_trace *trace);
-extern int save_stack_trace_tsk_reliable(struct task_struct *tsk,
-struct stack_trace *trace);
-
-extern void print_stack_trace(struct stack_trace *trace, int spaces);
-extern int snprint_stack_trace(char *buf, size_t size,
- struct stack_trace *trace, int spaces);
extern void stack_trace_print(unsigned long *trace, unsigned int nr_entries,
int spaces);
@@ -43,20 +26,23 @@ extern unsigned int stack_trace_save_reg
extern unsigned int stack_trace_save_user(unsigned long *store,
unsigned int size,
unsigned int skipnr);
+/*
+ * The below is for stack trace internals and architecture
+ * implementations. Do not use in generic code.
+ */
+struct stack_trace {
+ unsigned int nr_entries, max_entries;
+ unsigned long *entries;
+ int skip; /* input argument: How many entries to skip */
+};
-#ifdef CONFIG_USER_STACKTRACE_SUPPORT
+extern void save_stack_trace(struct stack_trace *trace);
+extern void save_stack_trace_regs(struct pt_regs *regs,
+ struct stack_trace *trace);
+extern void save_stack_trace_tsk(struct task_struct *tsk,
+ struct stack_trace *trace);
+extern int save_stack_trace_tsk_reliable(struct task_struct *tsk,
+struct stack_trace *trace);
extern void save_stack_trace_user(struct stack_trace *trace);
-#else
-# define save_stack_trace_user(trace) do { } while (0)
-#endif
-
-#else /* !CONFIG_STACKTRACE */
-# define save_stack_trace(trace) do { } while (0)
-# define save_stack_trace_tsk(tsk, trace) do { } while (0)
-# define save_stack_trace_user(trace) do { } while (0)
-# define print_stack_trace(trace, spaces) do { } while (0)
-# define snprint_stack_trace(buf, size, trace, spaces) do { } while (0)
-# define save_stack_trace_tsk_reliable(tsk, trace) ({ -ENOSYS; })
-#endif /* CONFIG_STACKTRACE */
#endif /* __LINUX_STACKTRACE_H */
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -30,12 +30,6 @@ void stack_trace_print(unsigned long *en
}
EXPORT_SYMBOL_GPL(stack_trace_print);
-void print_stack_trace(struct stack_trace *trace, int spaces)
-{
- stack_trace_print(trace->entries, trace->nr_entries, spaces);
-}
-EXPORT_SYMBOL_GPL(print_stack_trace);
-
/**
* stack_trace_snprint - Print the entries in the stack trace into a buffer
* @buf: Pointer to the print buffer
@@ -70,14 +64,6 @@ int stack_trace_snprint(char *buf, size_
}
EXPORT_SYMBOL_GPL(stack_trace_snprint);
-int snprint_stack_trace(char *buf, size_t size,
- struct stack_trace *trace, int spaces)
-{
- return stack_trace_snprint(buf, size, trace->entries,
- trace->nr_entries, spaces);
-}
-EXPORT_SYMBOL_GPL(snprint_stack_trace);
-
/*
* Architectures that do not implement save_stack_trace_*()
* get these weak aliases and once-per-bootup warnings
